Thread (2 messages) 2 messages, 2 authors, 2016-02-17

[PATCH v4 5/8] [Media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver

From: Hans Verkuil <hidden>
Date: 2016-02-17 07:48:09
Also in: linux-devicetree, linux-media, linux-mediatek, lkml

On 02/16/2016 07:37 AM, tiffany lin wrote:
quoted
quoted
+static int fops_vcodec_open(struct file *file)
+{
+	struct video_device *vfd = video_devdata(file);
+	struct mtk_vcodec_dev *dev = video_drvdata(file);
+	struct mtk_vcodec_ctx *ctx = NULL;
+	int ret = 0;
+
+	mutex_lock(&dev->dev_mutex);
+
+	ctx = devm_kzalloc(&dev->plat_dev->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx) {
+		ret = -ENOMEM;
+		goto err_alloc;
+	}
+
+	if (dev->num_instances >= MTK_VCODEC_MAX_ENCODER_INSTANCES) {
+		mtk_v4l2_err("Too many open contexts\n");
+		ret = -EBUSY;
+		goto err_no_ctx;
Hmm. I never like it if you can't open a video node because of a reason like this.

I.e. a simple 'v4l2-ctl -D' (i.e. calling QUERYCAP) should never fail.

If there are hardware limitation that prevent more than X instances from running at
the same time, then those limitations typically kick in when you start to stream
(or possibly when calling REQBUFS). But before that it should always be possible to
open the device.

Having this check at open() is an indication of a poor design.

Is this is a hardware limitation at all?
This is to make sure performance meet requirements, such as bitrate and
framerate.
Is it the driver's job to make enforce this? What if the application only
deals with low-res video, but wants to encode a lot of those? Or is encoding
a video off-line?

The driver generally doesn't know the use-case, so if this is an artificial
limitation as opposed to a hardware limitation, then I would just drop this.

Regards,

	Hans
We got your point. We will remove this and move limitation control to
start_streaming or REQBUFS.
Appreciated for your suggestion.:)

quoted
quoted
+	}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help