RE: [PATCH v4 06/14] v4l: ti-vpe: Fix some params in VPE data descriptors
Hi,
> From: Archit Taneja [mailto:arc...@ti.com]
> Sent: Thursday, March 13, 2014 12:44 PM
> To: k.deb...@samsung.com; hverk...@xs4all.nl
> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Archit
> Taneja
> Subject: [PATCH v4 06/14] v4l: ti-vpe: Fix some params in VPE data
> descriptors
>
> Some parameters of the VPE descriptors were understood incorrectly.
> They are now fixed. The fixes are explained as follows:
>
> - When adding an inbound data descriptor to the VPDMA descriptor list,
> we intend
> to use c_rect as the cropped region fetched by VPDMA. Therefore,
> c_rect->width
> shouldn't be used to calculate the line stride, the original image
> width
> should be used for that. We add a 'width' argument which gives the
> buffer
> width in memory.
>
> - frame_width and frame_height describe the complete width and height
> of the
> client to which the channel is connected. If there are multiple
> channels
> fetching data and providing to the same client, the above 2 arguments
> should
> be the width and height of the region covered by all the channels. In
> the case
> where there is only one channel providing pixel data to the client
> (like in VPE), frame_width and frame_height should be the cropped
> width and
> cropped height respectively. The calculation of these params is done
> in the
> vpe driver now.
>
> - start_h and start_v is also used in the case of multiple channels to
> describe
> where each channel should start filling pixel data. We don't use this
> in VPE,
> and pass 0s to the vpdma_add_in_dtd() helper.
>
> - Some minor changes are made to the vpdma_add_out_dtd() helper. The
> c_rect
> param is used for specifying the 'composition' target, and 'width'
> is added
> to calculate the line stride.
This patch looks ok. Passes checkpatch and compiles. I can't say much more
as I have limited knowledge of the internals of VPE and don't have the
hardware.
> Signed-off-by: Archit Taneja
Acked-by: Kamil Debski
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
> ---
> drivers/media/platform/ti-vpe/vpdma.c | 60
> +++
> drivers/media/platform/ti-vpe/vpdma.h | 10 +++---
> drivers/media/platform/ti-vpe/vpe.c | 18 +++
> 3 files changed, 64 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/platform/ti-vpe/vpdma.c
> b/drivers/media/platform/ti-vpe/vpdma.c
> index 73dd38e..a51a013 100644
> --- a/drivers/media/platform/ti-vpe/vpdma.c
> +++ b/drivers/media/platform/ti-vpe/vpdma.c
> @@ -614,8 +614,17 @@ static void dump_dtd(struct vpdma_dtd *dtd)
> /*
> * append an outbound data transfer descriptor to the given descriptor
> list,
> * this sets up a 'client to memory' VPDMA transfer for the given
> VPDMA channel
> + *
> + * @list: vpdma desc list to which we add this decriptor
> + * @width: width of the image in pixels in memory
> + * @c_rect: compose params of output image
> + * @fmt: vpdma data format of the buffer
> + * dma_addr: dma address as seen by VPDMA
> + * chan: VPDMA channel
> + * flags: VPDMA flags to configure some descriptor fileds
> */
> -void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect
> *c_rect,
> +void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width,
> + const struct v4l2_rect *c_rect,
> const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
> enum vpdma_channel chan, u32 flags)
> {
> @@ -623,6 +632,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list,
> struct v4l2_rect *c_rect,
> int field = 0;
> int notify = 1;
> int channel, next_chan;
> + struct v4l2_rect rect = *c_rect;
> int depth = fmt->depth;
> int stride;
> struct vpdma_dtd *dtd;
> @@ -630,11 +640,15 @@ void vpdma_add_out_dtd(struct vpdma_desc_list
> *list, struct v4l2_rect *c_rect,
> channel = next_chan = chan_info[chan].num;
>
> if (fmt->type == VPDMA_DATA_FMT_TYPE_YUV &&
> - fmt->data_type == DATA_TYPE_C420)
> + fmt->data_type == DATA_TYPE_C420) {
> + rect.height >>= 1;
> + rect.top >>= 1;
> depth = 8;
> + }
>
> - stride = ALIGN((depth * c_rect->width) >> 3, VPDMA_STRIDE_ALIGN);
> - dma_addr += (c_rect->left * depth) >> 3;
> + stride = ALIGN((depth * width) >> 3, VPDMA_STRIDE_ALIGN);
> +
> + dma_addr += rect.top * stride + (rect.left * depth >> 3);
>
> dtd = list->next;
> WARN_ON((void *)(dtd
RE: [PATCH v4 08/14] v4l: ti-vpe: Rename csc memory resource name
Hi,
> From: Archit Taneja [mailto:arc...@ti.com]
> Sent: Thursday, March 13, 2014 12:44 PM
>
> Rename the memory block resource "vpe_csc" to "csc" since it also
> exists within the VIP IP block. This would make the name more generic,
> and both VPE and VIP DT nodes in the future can use it.
I understand that this is not yet used in any public dts files. Right?
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
>
> Signed-off-by: Archit Taneja
> ---
> drivers/media/platform/ti-vpe/csc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/ti-vpe/csc.c
> b/drivers/media/platform/ti-vpe/csc.c
> index acfea50..039 100644
> --- a/drivers/media/platform/ti-vpe/csc.c
> +++ b/drivers/media/platform/ti-vpe/csc.c
> @@ -180,7 +180,7 @@ struct csc_data *csc_create(struct platform_device
> *pdev)
> csc->pdev = pdev;
>
> csc->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "vpe_csc");
> + "csc");
> if (csc->res == NULL) {
> dev_err(&pdev->dev, "missing platform resources data\n");
> return ERR_PTR(-ENODEV);
> --
> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/14] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
> From: Archit Taneja [mailto:arc...@ti.com] > Sent: Thursday, March 13, 2014 12:44 PM > > VPE has a ctrl parameter which decides how many mem to mem transactions > the active job from the job queue can perform. > > The driver's job_ready() made sure that the number of ready source > buffers are sufficient for the job to execute successfully. But it > didn't make sure if there are sufficient ready destination buffers in > the capture queue for the VPE output. > > If the time taken by VPE to process a single frame is really slow, then > it's possible that we don't need to imply such a restriction on the dst > queue, but really fast transactions(small resolution, no de-interlacing) > may cause us to hit the condition where we don't have any free buffers > for the VPE to write on. > > Add the extra check in job_ready() to make sure we have the sufficient > amount of destination buffers. > > Signed-off-by: Archit Taneja Acked-by: Kamil Debski > --- > drivers/media/platform/ti-vpe/vpe.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/ti-vpe/vpe.c > b/drivers/media/platform/ti-vpe/vpe.c > index 7a77a5b..f3143ac 100644 > --- a/drivers/media/platform/ti-vpe/vpe.c > +++ b/drivers/media/platform/ti-vpe/vpe.c > @@ -887,6 +887,9 @@ static int job_ready(void *priv) > if (v4l2_m2m_num_src_bufs_ready(ctx->m2m_ctx) < needed) > return 0; > > + if (v4l2_m2m_num_dst_bufs_ready(ctx->m2m_ctx) < needed) > + return 0; > + > return 1; > } > > -- > 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 01/14] v4l: ti-vpe: Make sure in job_ready that we have the needed number of dst_bufs
Hi, > From: Archit Taneja [mailto:arc...@ti.com] > Sent: Tuesday, March 11, 2014 9:34 AM > > VPE has a ctrl parameter which decides how many mem to mem transactions > the active job from the job queue can perform. > > The driver's job_ready() made sure that the number of ready source > buffers are sufficient for the job to execute successfully. But it > didn't make sure if there are sufficient ready destination buffers in > the capture queue for the VPE output. > > If the time taken by VPE to process a single frame is really slow, then > it's possible that we don't need to imply such a restriction on the dst > queue, but really fast transactions(small resolution, no de-interlacing) > may cause us to hit the condition where we don't have any free buffers > for the VPE to write on. > > Add the extra check in job_ready() to make sure we have the sufficient > amount of destination buffers. > > Signed-off-by: Archit Taneja Acked-by: Kamil Debski Best wishes, -- Kamil Debski Samsung R&D Institute Poland > --- > drivers/media/platform/ti-vpe/vpe.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/ti-vpe/vpe.c > b/drivers/media/platform/ti-vpe/vpe.c > index 7a77a5b..f3143ac 100644 > --- a/drivers/media/platform/ti-vpe/vpe.c > +++ b/drivers/media/platform/ti-vpe/vpe.c > @@ -887,6 +887,9 @@ static int job_ready(void *priv) > if (v4l2_m2m_num_src_bufs_ready(ctx->m2m_ctx) < needed) > return 0; > > + if (v4l2_m2m_num_dst_bufs_ready(ctx->m2m_ctx) < needed) > + return 0; > + > return 1; > } > > -- > 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 02/14] v4l: ti-vpe: register video device only when firmware is loaded
Hi, > From: Archit Taneja [mailto:arc...@ti.com] > Sent: Thursday, March 13, 2014 1:09 PM > > Hi Kamil, > > On Thursday 13 March 2014 05:18 PM, Kamil Debski wrote: > > Hi Archit, > > > >> From: Archit Taneja [mailto:arc...@ti.com] > >> Sent: Tuesday, March 11, 2014 9:34 AM > >> > >> vpe fops(vpe_open in particular) should be called only when VPDMA > >> firmware is loaded. File operations on the video device are possible > >> the moment it is registered. > >> > >> Currently, we register the video device for VPE at driver probe, > >> after calling a vpdma helper to initialize VPDMA and load firmware. > >> This function is non-blocking(it calls request_firmware_nowait()), > >> and doesn't ensure that the firmware is actually loaded when it > returns. > >> > >> We remove the device registration from vpe probe, and move it to a > >> callback provided by the vpe driver to the vpdma library, through > >> vpdma_create(). > >> > >> The ready field in vpdma_data is no longer needed since we always > >> have firmware loaded before the device is registered. > >> > >> A minor problem with this approach is that if the > >> video_register_device fails(which doesn't really happen), the vpe > >> platform device would be registered. > >> however, there won't be any v4l2 device corresponding to it. > > > > Could you explain to me one thing. request_firmware cannot be used in > > probe, thus you are using request_firmware_nowait. Why cannot the > > firmware be loaded on open with a regular request_firmware that is > > waiting? > > I totally agree with you here. Placing the firmware in open() would > probably make more sense. > > The reason I didn't place it in open() is because I didn't want to > release firmware in a corresponding close(), and be in a situation > where the firmware is loaded multiple times in the driver's lifetime. Would it be possible to load firmware in open and release it in remove? I know that this would disturb the symmetry between open-release and probe-remove. But this could work. > There are some reasons for doing this. First, it will require more > testing with respect to whether the firmware is loaded correctly > successive times :), the second is that loading firmware might probably > take a bit of time, and we don't want to make applications too slow(I > haven't measured the time taken, so I don't have a strong case for this > either) > > > > > This patch seems to swap one problem for another. The possibility > that > > open fails (because firmware is not yet loaded) is swapped for a > vague > > possibility that video_register_device. > > The driver will work fine in most cases even without this patch(apart > from the possibility mentioned as above). > > We could discard this patch from this series, and I can work on a patch > which moves firmware loading to the vpe_open() call, and hence solving > the issue in the right manner. Does that sound fine? I agree, this should be a good solution. > > Thanks, > Archit Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 02/14] v4l: ti-vpe: register video device only when firmware is loaded
Hi Archit,
> From: Archit Taneja [mailto:arc...@ti.com]
> Sent: Tuesday, March 11, 2014 9:34 AM
>
> vpe fops(vpe_open in particular) should be called only when VPDMA
> firmware is loaded. File operations on the video device are possible
> the moment it is registered.
>
> Currently, we register the video device for VPE at driver probe, after
> calling a vpdma helper to initialize VPDMA and load firmware. This
> function is non-blocking(it calls request_firmware_nowait()), and
> doesn't ensure that the firmware is actually loaded when it returns.
>
> We remove the device registration from vpe probe, and move it to a
> callback provided by the vpe driver to the vpdma library, through
> vpdma_create().
>
> The ready field in vpdma_data is no longer needed since we always have
> firmware loaded before the device is registered.
>
> A minor problem with this approach is that if the video_register_device
> fails(which doesn't really happen), the vpe platform device would be
> registered.
> however, there won't be any v4l2 device corresponding to it.
Could you explain to me one thing. request_firmware cannot be used in
probe, thus you are using request_firmware_nowait. Why cannot the firmware
be
loaded on open with a regular request_firmware that is waiting?
This patch seems to swap one problem for another. The possibility that open
fails (because firmware is not yet loaded) is swapped for a vague
possibility
that video_register_device.
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
>
> Signed-off-by: Archit Taneja
> ---
> drivers/media/platform/ti-vpe/vpdma.c | 8 +++--
> drivers/media/platform/ti-vpe/vpdma.h | 7 +++--
> drivers/media/platform/ti-vpe/vpe.c | 55 ---
>
> 3 files changed, 41 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/platform/ti-vpe/vpdma.c
> b/drivers/media/platform/ti-vpe/vpdma.c
> index e8175e7..73dd38e 100644
> --- a/drivers/media/platform/ti-vpe/vpdma.c
> +++ b/drivers/media/platform/ti-vpe/vpdma.c
> @@ -781,7 +781,7 @@ static void vpdma_firmware_cb(const struct firmware
> *f, void *context)
> /* already initialized */
> if (read_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK,
> VPDMA_LIST_RDY_SHFT)) {
> - vpdma->ready = true;
> + vpdma->cb(vpdma->pdev);
> return;
> }
>
> @@ -811,7 +811,7 @@ static void vpdma_firmware_cb(const struct firmware
> *f, void *context)
> goto free_buf;
> }
>
> - vpdma->ready = true;
> + vpdma->cb(vpdma->pdev);
>
> free_buf:
> vpdma_unmap_desc_buf(vpdma, &fw_dma_buf); @@ -839,7 +839,8 @@
> static int vpdma_load_firmware(struct vpdma_data *vpdma)
> return 0;
> }
>
> -struct vpdma_data *vpdma_create(struct platform_device *pdev)
> +struct vpdma_data *vpdma_create(struct platform_device *pdev,
> + void (*cb)(struct platform_device *pdev))
> {
> struct resource *res;
> struct vpdma_data *vpdma;
> @@ -854,6 +855,7 @@ struct vpdma_data *vpdma_create(struct
> platform_device *pdev)
> }
>
> vpdma->pdev = pdev;
> + vpdma->cb = cb;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vpdma");
> if (res == NULL) {
> diff --git a/drivers/media/platform/ti-vpe/vpdma.h
> b/drivers/media/platform/ti-vpe/vpdma.h
> index cf40f11..bf5f8bb 100644
> --- a/drivers/media/platform/ti-vpe/vpdma.h
> +++ b/drivers/media/platform/ti-vpe/vpdma.h
> @@ -35,8 +35,8 @@ struct vpdma_data {
>
> struct platform_device *pdev;
>
> - /* tells whether vpdma firmware is loaded or not */
> - bool ready;
> + /* callback to VPE driver when the firmware is loaded */
> + void (*cb)(struct platform_device *pdev);
> };
>
> enum vpdma_data_format_type {
> @@ -208,6 +208,7 @@ void vpdma_set_frame_start_event(struct vpdma_data
> *vpdma, void vpdma_dump_regs(struct vpdma_data *vpdma);
>
> /* initialize vpdma, passed with VPE's platform device pointer */ -
> struct vpdma_data *vpdma_create(struct platform_device *pdev);
> +struct vpdma_data *vpdma_create(struct platform_device *pdev,
> + void (*cb)(struct platform_device *pdev));
>
> #endif
> diff --git a/drivers/media/platform/ti-vpe/vpe.c
> b/drivers/media/platform/ti-vpe/vpe.c
> index f3143ac..f1eae67 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -1817,11 +1817,6 @@ static int vpe_open(struct file *file)
>
> vpe_dbg(dev, "vpe_open\n");
>
> - if (!dev->vpdma->ready) {
> -
RE: [PATCH v2 7/7] v4l: ti-vpe: Add selection API in VPE driver
Hi Archit,
> From: Archit Taneja [mailto:arc...@ti.com]
> Sent: Tuesday, March 04, 2014 12:25 PM
>
> Hi,
>
> On Tuesday 04 March 2014 03:10 PM, Hans Verkuil wrote:
> > Hi Archit,
> >
> > On 03/04/14 09:49, Archit Taneja wrote:
> >> Add selection ioctl ops. For VPE, cropping makes sense only for the
> >> input to VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and
> >> composing makes sense only for the output of VPE(or
> V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
> >>
> >> For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing
> the
> >> output in a rectangle within the capture buffer. For the OUTPUT
> type,
> >> V4L2_SEL_TGT_CROP results in selecting a rectangle region within the
> source buffer.
> >>
> >> Setting the crop/compose rectangles should successfully result in
> >> re-configuration of registers which are affected when either source
> >> or destination dimensions change, set_srcdst_params() is called for
> this purpose.
> >>
> >> Signed-off-by: Archit Taneja
> >> ---
> >> drivers/media/platform/ti-vpe/vpe.c | 142
>
> >> 1 file changed, 142 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/ti-vpe/vpe.c
> >> b/drivers/media/platform/ti-vpe/vpe.c
> >> index 03a6846..b938590 100644
> >> --- a/drivers/media/platform/ti-vpe/vpe.c
> >> +++ b/drivers/media/platform/ti-vpe/vpe.c
> >> @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct
> vpe_ctx *ctx,
> >> {
> >>switch (type) {
> >>case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> >>return &ctx->q_data[Q_DATA_SRC];
> >>case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> >
> > I noticed that the querycap implementation is wrong. It reports
> > V4L2_CAP_VIDEO_M2M instead of V4L2_CAP_VIDEO_M2M_MPLANE.
> >
> > This driver is using the multiplanar formats, so the M2M_MPLANE cap
> > should be set.
> >
> > This should be a separate patch.
>
> Thanks for pointing this out, I'll make a patch for that.
>
> >
> > BTW, did you test the driver with the v4l2-compliance tool? The
> latest
> > version
> > (http://git.linuxtv.org/v4l-utils.git) has m2m support.
> >
>
> I haven't tested it with this yet.
>
> > However, if you want to test streaming (the -s option), then you will
> > probably need to base your kernel on this tree:
> >
> >
> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2
> > -part1
>
> I can give it a try. It'll probably take a bit more time to try this
> out. I'll need to port some minor DRA7x stuff.
>
> Kamil,
>
> Do you think you have some more time for the m2m pull request?
Yes, I would like to send the final pull request at the beginning of
the coming week.
> >
> > That branch contains a pile of fixes for vb2 and without that
> > v4l2-compliance -s will fail a number of tests.
> >
> >>return &ctx->q_data[Q_DATA_DST];
> >>default:
> >>BUG();
> >> @@ -1585,6 +1587,143 @@ static int vpe_s_fmt(struct file *file, void
> *priv, struct v4l2_format *f)
> >>return set_srcdst_params(ctx);
> >> }
> >>
> >> +static int __vpe_try_selection(struct vpe_ctx *ctx, struct
> >> +v4l2_selection *s) {
> >> + struct vpe_q_data *q_data;
> >> +
> >> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
> >> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
> >> + return -EINVAL;
> >> +
> >> + q_data = get_q_data(ctx, s->type);
> >> + if (!q_data)
> >> + return -EINVAL;
> >> +
> >> + switch (s->target) {
> >> + case V4L2_SEL_TGT_COMPOSE:
> >> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> >> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> >> + /*
> >> + * COMPOSE target is only valid for capture buffer type,
> for
> >> + * output buffer type, assign existing crop parameters to
> the
> >> + * selection rectangle
> >> + */
> >> + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> >> + break;
> >> + } else {
> >
> > No need for the 'else' keywork here.
> >
> >> + s->
RE: [PATCH 7/7] v4l: ti-vpe: Add crop support in VPE driver
Hi Archit, > From: Archit Taneja [mailto:arc...@ti.com] > Sent: Monday, March 03, 2014 9:26 AM > > Hi, > > On Monday 03 March 2014 01:20 PM, Hans Verkuil wrote: > > Hi Archit! > > > > On 03/03/2014 08:33 AM, Archit Taneja wrote: > >> Add crop ioctl ops. For VPE, cropping only makes sense with the > input > >> to VPE, or the V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE buffer type. > >> > >> For the CAPTURE type, a S_CROP ioctl results in setting the crop > >> region as the whole image itself, hence making crop dimensions same > as the pix dimensions. > >> > >> Setting the crop successfully should result in re-configuration of > >> those registers which are affected when either source or destination > >> dimensions change, set_srcdst_params() is called for this purpose. > >> > >> Some standard crop parameter checks are done in __vpe_try_crop(). > > > > Please use the selection ops instead: if you implement cropping with > > those then you'll support both the selection API and the old cropping > > API will be implemented by the v4l2 core using the selection ops. Two > for the price of one... > > > > Thanks for the feedback. I'll use selection ops here. >From your reply I understand that you will send a v2 of these patches, right? If so, please correct the typos I mentioned in the patch 5/7. Also, it is quite late for v3.15. I did already send a pull request to Mauro, However I have one patch pending. Could you tell me when are you planning to post v2 of these patches? I want to decide whether should I wait for your patchset or should I send the second pull request with the pending patch as soon as possible. Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/7] v4l: ti-vpe: Allow usage of smaller images
Hi Archit, > From: Archit Taneja [mailto:arc...@ti.com] > Sent: Monday, March 03, 2014 8:33 AM > > The minimum width and height for VPE input/output was kept as 128 > pixels. VPE doesn't have a constraint on the image height, it requires > the image width to be atleast 16 bytes. "16 bytes" - shouldn't it be pixels? (also "at least" :) ) I can correct it when applying the patch if the above is the case. > Change the minimum supported dimensions to 32x32. This allows us to de- > interlace qcif content. A smaller image size than 32x32 didn't make > much sense, so stopped at this. > > Signed-off-by: Archit Taneja Best wishes, -- Kamil Debski Samsung R&D Institute Poland > --- > drivers/media/platform/ti-vpe/vpe.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/ti-vpe/vpe.c > b/drivers/media/platform/ti-vpe/vpe.c > index 915029b..3a610a6 100644 > --- a/drivers/media/platform/ti-vpe/vpe.c > +++ b/drivers/media/platform/ti-vpe/vpe.c > @@ -49,8 +49,8 @@ > #define VPE_MODULE_NAME "vpe" > > /* minimum and maximum frame sizes */ > -#define MIN_W128 > -#define MIN_H128 > +#define MIN_W32 > +#define MIN_H32 > #define MAX_W1920 > #define MAX_H1080 > > -- > 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 3/4] v4l: ti-vpe: Add VPE mem to mem driver
Hi Archit, Please find my comment below. > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > Sent: Tuesday, October 15, 2013 3:52 PM > > Kamil, > > Can you take this driver as m2m maintainer or should I take it? > > Regards, > > Hans > > On 10/15/2013 03:47 PM, Archit Taneja wrote: > > Hi Hans, > > > > On Friday 11 October 2013 01:16 PM, Hans Verkuil wrote: > >> On 10/09/2013 04:29 PM, Archit Taneja wrote: > >>> VPE is a block which consists of a single memory to memory path > >>> which can perform chrominance up/down sampling, de-interlacing, > >>> scaling, and color space conversion of raster or tiled YUV420 > >>> coplanar, YUV422 coplanar or YUV422 interleaved video formats. > >>> > >>> We create a mem2mem driver based primarily on the mem2mem-testdev > example. > >>> The de-interlacer, scaler and color space converter are all > bypassed > >>> for now to keep the driver simple. Chroma up/down sampler blocks > are > >>> implemented, so conversion beteen different YUV formats is possible. > >>> > >>> Each mem2mem context allocates a buffer for VPE MMR values which it > >>> will use when it gets access to the VPE HW via the mem2mem queue, > it > >>> also allocates a VPDMA descriptor list to which configuration and > data descriptors are added. > >>> > >>> Based on the information received via v4l2 ioctls for the source > and > >>> destination queues, the driver configures the values for the MMRs, > >>> and stores them in the buffer. There are also some VPDMA parameters > >>> like frame start and line mode which needs to be configured, these > >>> are configured by direct register writes via the VPDMA helper > functions. > >>> > >>> The driver's device_run() mem2mem op will add each descriptor based > >>> on how the source and destination queues are set up for the given > >>> ctx, once the list is prepared, it's submitted to VPDMA, these > >>> descriptors when parsed by VPDMA will upload MMR registers, start > >>> DMA of video buffers on the various input and output clients/ports. > >>> > >>> When the list is parsed completely(and the DMAs on all the output > >>> ports done), an interrupt is generated which we use to notify that > >>> the source and destination buffers are done. > >>> > >>> The rest of the driver is quite similar to other mem2mem drivers, > we > >>> use the multiplane v4l2 ioctls as the HW support coplanar formats. > >>> > >>> Signed-off-by: Archit Taneja > >> > >> Acked-by: Hans Verkuil > >> > > > > Thanks for the Acks. Is it possible to queue these for 3.13? Yep, it is possible. But [v4,4/4] v4l: ti-vpe: Add de-interlacer support in VPE does not apply after applying [v5,3/4] v4l: ti-vpe: Add VPE mem to mem driver. Please send a v5 with all patches. Best wishes, Kamil -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 3/4] v4l: ti-vpe: Add VPE mem to mem driver
Hi Hans, Now, I am a bit busy with... USB. I have to admit I have a backlog of patches to look through and prepare a tree for Mauro. I wanted to do this on Thursday or Friday. Is it ok? BTW if you see any m2m patches in patchwork feel free to delegate them to me. Best wishes, -- Kamil Debski Linux Kernel Developer Samsung R&D Institute Poland > -Original Message- > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > Sent: Tuesday, October 15, 2013 3:52 PM > To: Kamil Debski > Cc: Archit Taneja; linux-me...@vger.kernel.org; linux- > o...@vger.kernel.org; laurent.pinch...@ideasonboard.com > Subject: Re: [PATCH v5 3/4] v4l: ti-vpe: Add VPE mem to mem driver > > Kamil, > > Can you take this driver as m2m maintainer or should I take it? > > Regards, > > Hans > > On 10/15/2013 03:47 PM, Archit Taneja wrote: > > Hi Hans, > > > > On Friday 11 October 2013 01:16 PM, Hans Verkuil wrote: > >> On 10/09/2013 04:29 PM, Archit Taneja wrote: > >>> VPE is a block which consists of a single memory to memory path > >>> which can perform chrominance up/down sampling, de-interlacing, > >>> scaling, and color space conversion of raster or tiled YUV420 > >>> coplanar, YUV422 coplanar or YUV422 interleaved video formats. > >>> > >>> We create a mem2mem driver based primarily on the mem2mem-testdev > example. > >>> The de-interlacer, scaler and color space converter are all > bypassed > >>> for now to keep the driver simple. Chroma up/down sampler blocks > are > >>> implemented, so conversion beteen different YUV formats is possible. > >>> > >>> Each mem2mem context allocates a buffer for VPE MMR values which it > >>> will use when it gets access to the VPE HW via the mem2mem queue, > it > >>> also allocates a VPDMA descriptor list to which configuration and > data descriptors are added. > >>> > >>> Based on the information received via v4l2 ioctls for the source > and > >>> destination queues, the driver configures the values for the MMRs, > >>> and stores them in the buffer. There are also some VPDMA parameters > >>> like frame start and line mode which needs to be configured, these > >>> are configured by direct register writes via the VPDMA helper > functions. > >>> > >>> The driver's device_run() mem2mem op will add each descriptor based > >>> on how the source and destination queues are set up for the given > >>> ctx, once the list is prepared, it's submitted to VPDMA, these > >>> descriptors when parsed by VPDMA will upload MMR registers, start > >>> DMA of video buffers on the various input and output clients/ports. > >>> > >>> When the list is parsed completely(and the DMAs on all the output > >>> ports done), an interrupt is generated which we use to notify that > >>> the source and destination buffers are done. > >>> > >>> The rest of the driver is quite similar to other mem2mem drivers, > we > >>> use the multiplane v4l2 ioctls as the HW support coplanar formats. > >>> > >>> Signed-off-by: Archit Taneja > >> > >> Acked-by: Hans Verkuil > >> > > > > Thanks for the Acks. Is it possible to queue these for 3.13? > > > > Archit > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework
Hi Kishon,
A small fix follows inline.
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Kishon Vijay Abraham I
> Sent: Friday, July 26, 2013 2:49 PM
>
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference
> to the PHY with or without using phandle. For dt-boot, the PHY drivers
> should also register *PHY provider* with the framework.
>
> PHY drivers should create the PHY by passing id and ops like init, exit,
> power_on and power_off. This framework is also pm runtime enabled.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for dt binding can be found
> at Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Cc: Tomasz Figa
> Cc: Greg Kroah-Hartman
> Signed-off-by: Kishon Vijay Abraham I
> Acked-by: Felipe Balbi
> Tested-by: Sylwester Nawrocki
> ---
> .../devicetree/bindings/phy/phy-bindings.txt | 66 ++
> Documentation/phy.txt | 166 +
> MAINTAINERS|8 +
> drivers/Kconfig|2 +
> drivers/Makefile |2 +
> drivers/phy/Kconfig| 18 +
> drivers/phy/Makefile |5 +
> drivers/phy/phy-core.c | 714
>
> include/linux/phy/phy.h| 270
> 9 files changed, 1251 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-
> bindings.txt
> create mode 100644 Documentation/phy.txt create mode 100644
> drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create
> mode 100644 drivers/phy/phy-core.c create mode 100644
> include/linux/phy/phy.h
>
[snip]
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h new file
> mode 100644 index 000..e444b23
> --- /dev/null
> +++ b/include/linux/phy/phy.h
> @@ -0,0 +1,270 @@
[snip]
> +struct phy_init_data {
> + unsigned int num_consumers;
> + struct phy_consumer *consumers;
> +};
> +
> +#define PHY_CONSUMER(_dev_name, _port) \
> +{\
> + .dev_name = _dev_name,\
> + .port = _port,\
> +}
> +
> +#define to_phy(dev) (container_of((dev), struct phy, dev))
> +
> +#define of_phy_provider_register(dev, xlate)\
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +
> +#define devm_of_phy_provider_register(dev, xlate) \
> + __of_phy_provider_register((dev), THIS_MODULE, (xlate))
I think this should be:
+ __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
Right?
> +
> +static inline void phy_set_drvdata(struct phy *phy, void *data) {
> + dev_set_drvdata(&phy->dev, data);
> +}
> +
> +static inline void *phy_get_drvdata(struct phy *phy) {
> + return dev_get_drvdata(&phy->dev);
> +}
> +
[snip]
Best wishes,
--
Kamil Debski
Linux Kernel Developer
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
