Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
Hi Hardik,
According to earlier mail from Sergio, I could be noticed that it is
totally ok to restrict number of buffers by driver. As matter of fact,
I was always considering about the H/W restriction and could find the
answer from his mail.
By the way, thank you for your answer.
Cheers,
Nate
On Tue, May 19, 2009 at 2:28 PM, Shah, Hardik wrote:
> Hi Nate,
>
>
>> -Original Message-
>> From: Aguirre Rodriguez, Sergio Alberto
>> Sent: Tuesday, May 19, 2009 10:52 AM
>> To: Dongsoo, Nathaniel Kim; Shah, Hardik
>> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
>> R;
>> Hiremath, Vaibhav
>> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>>
>> Hi Nate,
>>
>> I have 1 input regarding your question:
>>
>> >From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org]
>> On Behalf Of Dongsoo, Nathaniel Kim [dongsoo@gmail.com]
>> >Sent: Tuesday, May 19, 2009 7:53 AM
>> >To: Shah, Hardik
>> >Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
>> R; Hiremath, Vaibhav
>> >Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>> >
>> >Hello Hardik,
>> >
>> >Reviewing your driver, I found something made me curious.
>> >
>> >
>> >On Wed, Apr 22, 2009 at 3:25 PM, Hardik Shah wrote:
>>
>>
>>
>> >> +/* Buffer setup function is called by videobuf layer when REQBUF ioctl is
>> >> + * called. This is used to setup buffers and return size and count of
>> >> + * buffers allocated. After the call to this buffer, videobuf layer will
>> >> + * setup buffer queue depending on the size and count of buffers
>> >> + */
>> >> +static int omap_vout_buffer_setup(struct videobuf_queue *q, unsigned int
>> *count,
>> >> + unsigned int *size)
>> >> +{
>> >> + struct omap_vout_device *vout = q->priv_data;
>> >> + int startindex = 0, i, j;
>> >> + u32 phy_addr = 0, virt_addr = 0;
>> >> +
>> >> + if (!vout)
>> >> + return -EINVAL;
>> >> +
>> >> + if (V4L2_BUF_TYPE_VIDEO_OUTPUT != q->type)
>> >> + return -EINVAL;
>> >> +
>> >> + startindex = (vout->vid == OMAP_VIDEO1) ?
>> >> + video1_numbuffers : video2_numbuffers;
>> >> + if (V4L2_MEMORY_MMAP == vout->memory && *count < startindex)
>> >> + *count = startindex;
>> >> +
>> >> + if ((rotation_enabled(vout->rotation)) && *count > 4)
>> >> + *count = 4;
>> >> +
>> >
>> >
>> >
>> >This seems to be weird to me. If user requests multiple buffers more
>> >than 4, user cannot recognize that the number of buffers requested is
>> >forced to change into 4. I'm not sure whether this could be serious or
>> >not, but it is obvious that user can have doubt about why if user have
>> >no information about the OMAP H/W.
>> >Is it really necessary to be configured to 4?
> [Shah, Hardik] Rotation requires the VRFB contexts and limited number of
> contexts are available. So maximum number of buffers is fixed to 4 when
> rotation is enabled.
>
> Thanks,
> Hardik
>> >
>> >
>> >Cheers,
>> >
>> >Nate
>> >
>>
>> We did a very similar thing on omap3 camera driver, not exactly by the number
>> of buffers requested, but more about checking if the bytesize of the total
>> requested buffers was superior to the MMU fixed sized page table size
>> capabilities to map that size, then we were limiting the number of buffers
>> accordingly (for keeping the page table size fixed).
>>
>> According to the v4l2 spec, changing the count value should be valid, and it
>> is the userspace app responsability to check the value again, to confirm how
>> many of the requested buffers are actually available.
>>
>> Regards,
>> Sergio
>
--
=
DongSoo, Nathaniel Kim
Engineer
Mobile S/W Platform Lab.
Digital Media & Communications R&D Centre
Samsung Electronics CO., LTD.
e-mail : dongsoo@gmail.com
dongsoo45@samsung.com
--
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 3/3] OMAP2/3 V4L2 Display Driver
Hi Nate,
> -Original Message-
> From: Aguirre Rodriguez, Sergio Alberto
> Sent: Tuesday, May 19, 2009 10:52 AM
> To: Dongsoo, Nathaniel Kim; Shah, Hardik
> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh R;
> Hiremath, Vaibhav
> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
> Hi Nate,
>
> I have 1 input regarding your question:
>
> >From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org]
> On Behalf Of Dongsoo, Nathaniel Kim [dongsoo@gmail.com]
> >Sent: Tuesday, May 19, 2009 7:53 AM
> >To: Shah, Hardik
> >Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
> R; Hiremath, Vaibhav
> >Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >
> >Hello Hardik,
> >
> >Reviewing your driver, I found something made me curious.
> >
> >
> >On Wed, Apr 22, 2009 at 3:25 PM, Hardik Shah wrote:
>
>
>
> >> +/* Buffer setup function is called by videobuf layer when REQBUF ioctl is
> >> + * called. This is used to setup buffers and return size and count of
> >> + * buffers allocated. After the call to this buffer, videobuf layer will
> >> + * setup buffer queue depending on the size and count of buffers
> >> + */
> >> +static int omap_vout_buffer_setup(struct videobuf_queue *q, unsigned int
> *count,
> >> + unsigned int *size)
> >> +{
> >> + struct omap_vout_device *vout = q->priv_data;
> >> + int startindex = 0, i, j;
> >> + u32 phy_addr = 0, virt_addr = 0;
> >> +
> >> + if (!vout)
> >> + return -EINVAL;
> >> +
> >> + if (V4L2_BUF_TYPE_VIDEO_OUTPUT != q->type)
> >> + return -EINVAL;
> >> +
> >> + startindex = (vout->vid == OMAP_VIDEO1) ?
> >> + video1_numbuffers : video2_numbuffers;
> >> + if (V4L2_MEMORY_MMAP == vout->memory && *count < startindex)
> >> + *count = startindex;
> >> +
> >> + if ((rotation_enabled(vout->rotation)) && *count > 4)
> >> + *count = 4;
> >> +
> >
> >
> >
> >This seems to be weird to me. If user requests multiple buffers more
> >than 4, user cannot recognize that the number of buffers requested is
> >forced to change into 4. I'm not sure whether this could be serious or
> >not, but it is obvious that user can have doubt about why if user have
> >no information about the OMAP H/W.
> >Is it really necessary to be configured to 4?
[Shah, Hardik] Rotation requires the VRFB contexts and limited number of
contexts are available. So maximum number of buffers is fixed to 4 when
rotation is enabled.
Thanks,
Hardik
> >
> >
> >Cheers,
> >
> >Nate
> >
>
> We did a very similar thing on omap3 camera driver, not exactly by the number
> of buffers requested, but more about checking if the bytesize of the total
> requested buffers was superior to the MMU fixed sized page table size
> capabilities to map that size, then we were limiting the number of buffers
> accordingly (for keeping the page table size fixed).
>
> According to the v4l2 spec, changing the count value should be valid, and it
> is the userspace app responsability to check the value again, to confirm how
> many of the requested buffers are actually available.
>
> Regards,
> Sergio
--
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 3/3] OMAP2/3 V4L2 Display Driver
Hi Sergio,
Thank you for your explanation . I learned much from that.
Cheers,
Nate
On Tue, May 19, 2009 at 2:22 PM, Aguirre Rodriguez, Sergio Alberto
wrote:
> Hi Nate,
>
> I have 1 input regarding your question:
>
>>From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org]
>>On Behalf Of Dongsoo, Nathaniel Kim [dongsoo@gmail.com]
>>Sent: Tuesday, May 19, 2009 7:53 AM
>>To: Shah, Hardik
>>Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
>>R; Hiremath, Vaibhav
>>Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>>
>>Hello Hardik,
>>
>>Reviewing your driver, I found something made me curious.
>>
>>
>>On Wed, Apr 22, 2009 at 3:25 PM, Hardik Shah wrote:
>
>
>
>>> +/* Buffer setup function is called by videobuf layer when REQBUF ioctl is
>>> + * called. This is used to setup buffers and return size and count of
>>> + * buffers allocated. After the call to this buffer, videobuf layer will
>>> + * setup buffer queue depending on the size and count of buffers
>>> + */
>>> +static int omap_vout_buffer_setup(struct videobuf_queue *q, unsigned int
>>> *count,
>>> + unsigned int *size)
>>> +{
>>> + struct omap_vout_device *vout = q->priv_data;
>>> + int startindex = 0, i, j;
>>> + u32 phy_addr = 0, virt_addr = 0;
>>> +
>>> + if (!vout)
>>> + return -EINVAL;
>>> +
>>> + if (V4L2_BUF_TYPE_VIDEO_OUTPUT != q->type)
>>> + return -EINVAL;
>>> +
>>> + startindex = (vout->vid == OMAP_VIDEO1) ?
>>> + video1_numbuffers : video2_numbuffers;
>>> + if (V4L2_MEMORY_MMAP == vout->memory && *count < startindex)
>>> + *count = startindex;
>>> +
>>> + if ((rotation_enabled(vout->rotation)) && *count > 4)
>>> + *count = 4;
>>> +
>>
>>
>>
>>This seems to be weird to me. If user requests multiple buffers more
>>than 4, user cannot recognize that the number of buffers requested is
>>forced to change into 4. I'm not sure whether this could be serious or
>>not, but it is obvious that user can have doubt about why if user have
>>no information about the OMAP H/W.
>>Is it really necessary to be configured to 4?
>>
>>
>>Cheers,
>>
>>Nate
>>
>
> We did a very similar thing on omap3 camera driver, not exactly by the number
> of buffers requested, but more about checking if the bytesize of the total
> requested buffers was superior to the MMU fixed sized page table size
> capabilities to map that size, then we were limiting the number of buffers
> accordingly (for keeping the page table size fixed).
>
> According to the v4l2 spec, changing the count value should be valid, and it
> is the userspace app responsability to check the value again, to confirm how
> many of the requested buffers are actually available.
>
> Regards,
> Sergio
--
=
DongSoo, Nathaniel Kim
Engineer
Mobile S/W Platform Lab.
Digital Media & Communications R&D Centre
Samsung Electronics CO., LTD.
e-mail : dongsoo@gmail.com
dongsoo45@samsung.com
--
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 3/3] OMAP2/3 V4L2 Display Driver
Hi Nate,
I have 1 input regarding your question:
>From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org] On
>Behalf Of Dongsoo, Nathaniel Kim [dongsoo@gmail.com]
>Sent: Tuesday, May 19, 2009 7:53 AM
>To: Shah, Hardik
>Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh R;
>Hiremath, Vaibhav
>Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
>Hello Hardik,
>
>Reviewing your driver, I found something made me curious.
>
>
>On Wed, Apr 22, 2009 at 3:25 PM, Hardik Shah wrote:
>> +/* Buffer setup function is called by videobuf layer when REQBUF ioctl is
>> + * called. This is used to setup buffers and return size and count of
>> + * buffers allocated. After the call to this buffer, videobuf layer will
>> + * setup buffer queue depending on the size and count of buffers
>> + */
>> +static int omap_vout_buffer_setup(struct videobuf_queue *q, unsigned int
>> *count,
>> + unsigned int *size)
>> +{
>> + struct omap_vout_device *vout = q->priv_data;
>> + int startindex = 0, i, j;
>> + u32 phy_addr = 0, virt_addr = 0;
>> +
>> + if (!vout)
>> + return -EINVAL;
>> +
>> + if (V4L2_BUF_TYPE_VIDEO_OUTPUT != q->type)
>> + return -EINVAL;
>> +
>> + startindex = (vout->vid == OMAP_VIDEO1) ?
>> + video1_numbuffers : video2_numbuffers;
>> + if (V4L2_MEMORY_MMAP == vout->memory && *count < startindex)
>> + *count = startindex;
>> +
>> + if ((rotation_enabled(vout->rotation)) && *count > 4)
>> + *count = 4;
>> +
>
>
>
>This seems to be weird to me. If user requests multiple buffers more
>than 4, user cannot recognize that the number of buffers requested is
>forced to change into 4. I'm not sure whether this could be serious or
>not, but it is obvious that user can have doubt about why if user have
>no information about the OMAP H/W.
>Is it really necessary to be configured to 4?
>
>
>Cheers,
>
>Nate
>
We did a very similar thing on omap3 camera driver, not exactly by the number
of buffers requested, but more about checking if the bytesize of the total
requested buffers was superior to the MMU fixed sized page table size
capabilities to map that size, then we were limiting the number of buffers
accordingly (for keeping the page table size fixed).
According to the v4l2 spec, changing the count value should be valid, and it is
the userspace app responsability to check the value again, to confirm how many
of the requested buffers are actually available.
Regards,
Sergio--
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 3/3] OMAP2/3 V4L2 Display Driver
Hi Inki,
I was on vacation just came,
Below are my comments.
> -Original Message-
> From: InKi Dae [mailto:daei...@gmail.com]
> Sent: Thursday, April 30, 2009 6:19 PM
> To: Shah, Hardik
> Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
> Thank you Shah, Hardik.
> I have patched your driver through that git.
>
> but your patch recognizes video2 layer as video1 device node in case
> that DSS2 has fb0 and fb1.
[Shah, Hardik] There is no need to change the nodes. If there is only one
video device present than it should be /dev/video1 and not /dev/video2. Its
upto the driver to use which video pipeline 1 or 2. So let the node
nomenclature be same.
Regards,
Hardik
>
> you said my patch will give rise to couple of more bugs related to
> global_alpha and pixel formats
> maybe that would be because of vout->vid_info.id = k + vout_count;
>
> so I have applied your patch to my way.
> this patch is more flexible and recognizes video2 layer correctly as
> video2 device node.
> --
>
> --- a/drivers/media/video/omap/omap_vout.c
> +++ b/drivers/media/video/omap/omap_vout.c
> @@ -2204,7 +2204,7 @@ free_buffers:
> /* Create video out devices */
> static int __init omap_vout_create_video_devices(struct platform_device
> *pdev)
> {
> - int r = 0, k;
> + int r = 0, k, vout_count;
> struct omap_vout_device *vout;
> struct video_device *vfd = NULL;
> struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
> @@ -2212,6 +2212,8 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
> struct omap2video_device *vid_dev = container_of(v4l2_dev, struct
> omap2video_device, v4l2_dev);
>
> + vout_count = 3 - pdev->num_resources;
> +
> for (k = 0; k < pdev->num_resources; k++) {
>
> vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
> @@ -2225,12 +2227,7 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
> vout->vid = k;
> vid_dev->vouts[k] = vout;
> vout->vid_dev = vid_dev;
> - /* Select video2 if only 1 overlay is controlled by V4L2 */
> - if (pdev->num_resources == 1)
> - vout->vid_info.overlays[0] = vid_dev->overlays[k + 2];
> - else
> - /* Else select video1 and video2 one by one. */
> - vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> + vout->vid_info.overlays[0] = vid_dev->overlays[k +
> vout_count];
> vout->vid_info.num_overlays = 1;
> vout->vid_info.id = k + 1;
> vid_dev->num_videos++;
> @@ -2253,7 +2250,7 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
> /* Register the Video device with V4L2
> */
> vfd = vout->vfd;
> - if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
> + if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
> vout_count) < 0) {
> printk(KERN_ERR VOUT_NAME ": could not register "
> "Video for Linux device\n");
> vfd->minor = -1
> --
> --
> -
>
> 2009/4/30 Shah, Hardik :
>
> >
> >> -Original Message-
> >> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> >> ow...@vger.kernel.org] On Behalf Of Shah, Hardik
> >> Sent: Thursday, April 30, 2009 11:51 AM
> >> To: InKi Dae
> >> Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> >> o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
> >> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >>
> >>
> >>
> >> > -Original Message-
> >> > From: InKi Dae [mailto:daei...@gmail.com]
> >> > Sent: Thursday, April 30, 2009 11:48 AM
> >> > To: Shah, Hardik
> >> > Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> >> > o...@vger.kernel.org; Jadav, Brijesh R; Hi
Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
Thank you Shah, Hardik.
I have patched your driver through that git.
but your patch recognizes video2 layer as video1 device node in case
that DSS2 has fb0 and fb1.
you said my patch will give rise to couple of more bugs related to
global_alpha and pixel formats
maybe that would be because of vout->vid_info.id = k + vout_count;
so I have applied your patch to my way.
this patch is more flexible and recognizes video2 layer correctly as
video2 device node.
--
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -2204,7 +2204,7 @@ free_buffers:
/* Create video out devices */
static int __init omap_vout_create_video_devices(struct platform_device *pdev)
{
- int r = 0, k;
+ int r = 0, k, vout_count;
struct omap_vout_device *vout;
struct video_device *vfd = NULL;
struct v4l2_device *v4l2_dev = platform_get_drvdata(pdev);
@@ -2212,6 +2212,8 @@ static int __init
omap_vout_create_video_devices(struct platform_device *pdev)
struct omap2video_device *vid_dev = container_of(v4l2_dev, struct
omap2video_device, v4l2_dev);
+ vout_count = 3 - pdev->num_resources;
+
for (k = 0; k < pdev->num_resources; k++) {
vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
@@ -2225,12 +2227,7 @@ static int __init
omap_vout_create_video_devices(struct platform_device *pdev)
vout->vid = k;
vid_dev->vouts[k] = vout;
vout->vid_dev = vid_dev;
- /* Select video2 if only 1 overlay is controlled by V4L2 */
- if (pdev->num_resources == 1)
- vout->vid_info.overlays[0] = vid_dev->overlays[k + 2];
- else
- /* Else select video1 and video2 one by one. */
- vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
+ vout->vid_info.overlays[0] = vid_dev->overlays[k + vout_count];
vout->vid_info.num_overlays = 1;
vout->vid_info.id = k + 1;
vid_dev->num_videos++;
@@ -2253,7 +2250,7 @@ static int __init
omap_vout_create_video_devices(struct platform_device *pdev)
/* Register the Video device with V4L2
*/
vfd = vout->vfd;
- if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
+ if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
vout_count) < 0) {
printk(KERN_ERR VOUT_NAME ": could not register "
"Video for Linux device\n");
vfd->minor = -1
-
2009/4/30 Shah, Hardik :
>
>> -Original Message-
>> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
>> ow...@vger.kernel.org] On Behalf Of Shah, Hardik
>> Sent: Thursday, April 30, 2009 11:51 AM
>> To: InKi Dae
>> Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
>> o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
>> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>>
>>
>>
>> > -Original Message-
>> > From: InKi Dae [mailto:daei...@gmail.com]
>> > Sent: Thursday, April 30, 2009 11:48 AM
>> > To: Shah, Hardik
>> > Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
>> > o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
>> > Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>> >
>> > hello Shah, Hardik..
>> >
>> > your omap_vout.c has the problem that it disables video1 or fb1.
>> > so I have modified your code.
>> >
>> > I defined and set platform_data for DSS2 in machine code.(or board file)
>> >
>> > static struct omapfb_platform_data xxx_dss_platform_data = {
>> > .mem_desc.region[0].format = OMAPFB_COLOR_ARGB32,
>> > .mem_desc.region[0].format_used=1,
>> >
>> > .mem_desc.region[1].format = OMAPFB_COLOR_RGB24U,
>> > .mem_desc.region[1].format_used=1,
>> >
>> > .mem_desc.region[2].format = OMAPFB_COLOR_ARGB32,
>> > .mem_desc.region[2].format_used=1,
>> > };
>> >
>> > omapfb_set_platform_data(&xxx_dss_platform_data);
>> >
>> > after that, omap_vout has resource count got referring to framebuffer
>> > count,
>> > registers overlay as vout&#x
RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Shah, Hardik
> Sent: Thursday, April 30, 2009 11:51 AM
> To: InKi Dae
> Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
> Subject: RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
>
>
> > -Original Message-
> > From: InKi Dae [mailto:daei...@gmail.com]
> > Sent: Thursday, April 30, 2009 11:48 AM
> > To: Shah, Hardik
> > Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> > o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
> > Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >
> > hello Shah, Hardik..
> >
> > your omap_vout.c has the problem that it disables video1 or fb1.
> > so I have modified your code.
> >
> > I defined and set platform_data for DSS2 in machine code.(or board file)
> >
> > static struct omapfb_platform_data xxx_dss_platform_data = {
> > .mem_desc.region[0].format = OMAPFB_COLOR_ARGB32,
> > .mem_desc.region[0].format_used=1,
> >
> > .mem_desc.region[1].format = OMAPFB_COLOR_RGB24U,
> > .mem_desc.region[1].format_used=1,
> >
> > .mem_desc.region[2].format = OMAPFB_COLOR_ARGB32,
> > .mem_desc.region[2].format_used=1,
> > };
> >
> > omapfb_set_platform_data(&xxx_dss_platform_data);
> >
> > after that, omap_vout has resource count got referring to framebuffer count,
> > registers overlay as vout's one and would decide to use which overlay.
> >
> > at that time, your code would face with impact on some overlay(fb or video).
> >
> > this patch would solve that problem.
> > when it sets overlay to vout, vout would get overlay array index to
> > avoid overlapping with other overlay.
> >
> >
> > sighed-off-by: InKi Dae.
> > ---
> > diff --git a/drivers/media/video/omap/omap_vout.c
> > b/drivers/media/video/omap/omap_vout.c
> > index 9b4a0d7..051298a 100644
> > --- a/drivers/media/video/omap/omap_vout.c
> > +++ b/drivers/media/video/omap/omap_vout.c
> > @@ -2246,11 +2246,13 @@ free_buffers:
> > /* Create video out devices */
> > static int __init omap_vout_create_video_devices(struct platform_device
> > *pdev)
> > {
> > - int r = 0, k;
> > + int r = 0, k, vout_count;
> > struct omap_vout_device *vout;
> > struct video_device *vfd = NULL;
> > struct omap2video_device *vid_dev = platform_get_drvdata(pdev);
> >
> > + vout_count = 3 - pdev->num_resources;
> > +
> > for (k = 0; k < pdev->num_resources; k++) {
> >
> > vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
> > @@ -2266,9 +2268,9 @@ static int __init
> > omap_vout_create_video_devices(struct platform_device *pdev)
> > vout->vid = k;
> > vid_dev->vouts[k] = vout;
> > vout->vid_info.vid_dev = vid_dev;
> > - vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> > + vout->vid_info.overlays[0] = vid_dev->overlays[k + vout_count];
> > vout->vid_info.num_overlays = 1;
> > - vout->vid_info.id = k + 1;
> > + vout->vid_info.id = k + vout_count;
> > vid_dev->num_videos++;
> >
> > /* Setup the default configuration for the video devices
> > @@ -2289,7 +2291,7 @@ static int __init
> > omap_vout_create_video_devices(struct platform_device *pdev)
> > /* Register the Video device with V4L2
> > */
> > vfd = vout->vfd;
> > - if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
> > + if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
> > vout_count) <
> > 0) {
> > printk(KERN_ERR VOUT_NAME ": could not register \
> > Video for Linux device\n");
> > vfd->minor = -1;
> >
> >
> > 2009/4/22 Shah, Hardik :
> [Shah, Hardik] Yes this is correct,
> I will apply this patch. I already found it and fixed it in different way but
> any way I will apply your patch.
[Shah, Hardik] Further on this inki. Solving this bug will give rise to couple
of more bugs related to global_alpha and pixel formats. That also is fixed. You
can refer
http://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git?p=people/vaibhav/ti-psp-omap-video.git;a=summary
> > >
>
RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> -Original Message-
> From: InKi Dae [mailto:daei...@gmail.com]
> Sent: Thursday, April 30, 2009 11:48 AM
> To: Shah, Hardik
> Cc: tomi.valkei...@nokia.com; linux-me...@vger.kernel.org; linux-
> o...@vger.kernel.org; Jadav, Brijesh R; Hiremath, Vaibhav
> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
> hello Shah, Hardik..
>
> your omap_vout.c has the problem that it disables video1 or fb1.
> so I have modified your code.
>
> I defined and set platform_data for DSS2 in machine code.(or board file)
>
> static struct omapfb_platform_data xxx_dss_platform_data = {
> .mem_desc.region[0].format = OMAPFB_COLOR_ARGB32,
> .mem_desc.region[0].format_used=1,
>
> .mem_desc.region[1].format = OMAPFB_COLOR_RGB24U,
> .mem_desc.region[1].format_used=1,
>
> .mem_desc.region[2].format = OMAPFB_COLOR_ARGB32,
> .mem_desc.region[2].format_used=1,
> };
>
> omapfb_set_platform_data(&xxx_dss_platform_data);
>
> after that, omap_vout has resource count got referring to framebuffer count,
> registers overlay as vout's one and would decide to use which overlay.
>
> at that time, your code would face with impact on some overlay(fb or video).
>
> this patch would solve that problem.
> when it sets overlay to vout, vout would get overlay array index to
> avoid overlapping with other overlay.
>
>
> sighed-off-by: InKi Dae.
> ---
> diff --git a/drivers/media/video/omap/omap_vout.c
> b/drivers/media/video/omap/omap_vout.c
> index 9b4a0d7..051298a 100644
> --- a/drivers/media/video/omap/omap_vout.c
> +++ b/drivers/media/video/omap/omap_vout.c
> @@ -2246,11 +2246,13 @@ free_buffers:
> /* Create video out devices */
> static int __init omap_vout_create_video_devices(struct platform_device
> *pdev)
> {
> - int r = 0, k;
> + int r = 0, k, vout_count;
> struct omap_vout_device *vout;
> struct video_device *vfd = NULL;
> struct omap2video_device *vid_dev = platform_get_drvdata(pdev);
>
> + vout_count = 3 - pdev->num_resources;
> +
> for (k = 0; k < pdev->num_resources; k++) {
>
> vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
> @@ -2266,9 +2268,9 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
> vout->vid = k;
> vid_dev->vouts[k] = vout;
> vout->vid_info.vid_dev = vid_dev;
> - vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> + vout->vid_info.overlays[0] = vid_dev->overlays[k + vout_count];
> vout->vid_info.num_overlays = 1;
> - vout->vid_info.id = k + 1;
> + vout->vid_info.id = k + vout_count;
> vid_dev->num_videos++;
>
> /* Setup the default configuration for the video devices
> @@ -2289,7 +2291,7 @@ static int __init
> omap_vout_create_video_devices(struct platform_device *pdev)
> /* Register the Video device with V4L2
>*/
> vfd = vout->vfd;
> - if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
> + if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
> vout_count) <
> 0) {
> printk(KERN_ERR VOUT_NAME ": could not register \
> Video for Linux device\n");
> vfd->minor = -1;
>
>
> 2009/4/22 Shah, Hardik :
[Shah, Hardik] Yes this is correct,
I will apply this patch. I already found it and fixed it in different way but
any way I will apply your patch.
> >
> >
> >> -Original Message-
> >> From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
> >> Sent: Wednesday, April 22, 2009 1:53 PM
> >> To: Shah, Hardik
> >> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
> R;
> >> Hiremath, Vaibhav
> >> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> >>
> >> Hi,
> >>
> >> On Wed, 2009-04-22 at 08:25 +0200, ext Hardik Shah wrote:
> >> > This is the version 5th of the Driver.
> >> >
> >> > Following are the features supported.
> >> > 1. Provides V4L2 user interface for the video pipelines of DSS
> >> > 2. Basic streaming working on LCD and TV.
> >> > 3. Support for various pixel formats like YUV, UYVY, RGB32, RGB24, RGB565
> >> > 4. Supports Alpha blending.
> >> > 5. Supports Color keying both source and destination.
> >> > 6. Supports rotation with RGB565 and RGB32 pixels formats.
> >> > 7. S
Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
hello Shah, Hardik..
your omap_vout.c has the problem that it disables video1 or fb1.
so I have modified your code.
I defined and set platform_data for DSS2 in machine code.(or board file)
static struct omapfb_platform_data xxx_dss_platform_data = {
.mem_desc.region[0].format = OMAPFB_COLOR_ARGB32,
.mem_desc.region[0].format_used=1,
.mem_desc.region[1].format = OMAPFB_COLOR_RGB24U,
.mem_desc.region[1].format_used=1,
.mem_desc.region[2].format = OMAPFB_COLOR_ARGB32,
.mem_desc.region[2].format_used=1,
};
omapfb_set_platform_data(&xxx_dss_platform_data);
after that, omap_vout has resource count got referring to framebuffer count,
registers overlay as vout's one and would decide to use which overlay.
at that time, your code would face with impact on some overlay(fb or video).
this patch would solve that problem.
when it sets overlay to vout, vout would get overlay array index to
avoid overlapping with other overlay.
sighed-off-by: InKi Dae.
---
diff --git a/drivers/media/video/omap/omap_vout.c
b/drivers/media/video/omap/omap_vout.c
index 9b4a0d7..051298a 100644
--- a/drivers/media/video/omap/omap_vout.c
+++ b/drivers/media/video/omap/omap_vout.c
@@ -2246,11 +2246,13 @@ free_buffers:
/* Create video out devices */
static int __init omap_vout_create_video_devices(struct platform_device *pdev)
{
- int r = 0, k;
+ int r = 0, k, vout_count;
struct omap_vout_device *vout;
struct video_device *vfd = NULL;
struct omap2video_device *vid_dev = platform_get_drvdata(pdev);
+ vout_count = 3 - pdev->num_resources;
+
for (k = 0; k < pdev->num_resources; k++) {
vout = kmalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
@@ -2266,9 +2268,9 @@ static int __init
omap_vout_create_video_devices(struct platform_device *pdev)
vout->vid = k;
vid_dev->vouts[k] = vout;
vout->vid_info.vid_dev = vid_dev;
- vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
+ vout->vid_info.overlays[0] = vid_dev->overlays[k + vout_count];
vout->vid_info.num_overlays = 1;
- vout->vid_info.id = k + 1;
+ vout->vid_info.id = k + vout_count;
vid_dev->num_videos++;
/* Setup the default configuration for the video devices
@@ -2289,7 +2291,7 @@ static int __init
omap_vout_create_video_devices(struct platform_device *pdev)
/* Register the Video device with V4L2
*/
vfd = vout->vfd;
- if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
+ if (video_register_device(vfd, VFL_TYPE_GRABBER, k +
vout_count) < 0) {
printk(KERN_ERR VOUT_NAME ": could not register \
Video for Linux device\n");
vfd->minor = -1;
2009/4/22 Shah, Hardik :
>
>
>> -Original Message-
>> From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com]
>> Sent: Wednesday, April 22, 2009 1:53 PM
>> To: Shah, Hardik
>> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh
>> R;
>> Hiremath, Vaibhav
>> Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
>>
>> Hi,
>>
>> On Wed, 2009-04-22 at 08:25 +0200, ext Hardik Shah wrote:
>> > This is the version 5th of the Driver.
>> >
>> > Following are the features supported.
>> > 1. Provides V4L2 user interface for the video pipelines of DSS
>> > 2. Basic streaming working on LCD and TV.
>> > 3. Support for various pixel formats like YUV, UYVY, RGB32, RGB24, RGB565
>> > 4. Supports Alpha blending.
>> > 5. Supports Color keying both source and destination.
>> > 6. Supports rotation with RGB565 and RGB32 pixels formats.
>> > 7. Supports cropping.
>> > 8. Supports Background color setting.
>> > 9. Works on latest DSS2 library from Tomi
>> > http://www.bat.org/~tomba/git/linux-omap-dss.git/
>> > 10. 1/4x scaling added. Detail testing left
>> >
>> > TODOS
>> > 1. Ioctls needs to be added for color space conversion matrix
>> > coefficient programming.
>> > 2. To be tested on DVI resolutions.
>> >
>> > Comments fixed from community.
>> > 1. V4L2 Driver for OMAP3/3 DSS.
>> > 2. Conversion of the custom ioctls to standard V4L2 ioctls like alpha
>> blending,
>> > color keying, rotation and back ground color setting
>> > 3. Re-organised the code as per community comments.
>> > 4. Added proper copyright year.
>> > 5. Added module name in printk
>> > 6. Kconfig option copy/paste error
>> > 7. Module
RE: [PATCH 3/3] OMAP2/3 V4L2 Display Driver
> -Original Message- > From: Tomi Valkeinen [mailto:tomi.valkei...@nokia.com] > Sent: Wednesday, April 22, 2009 1:53 PM > To: Shah, Hardik > Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh R; > Hiremath, Vaibhav > Subject: Re: [PATCH 3/3] OMAP2/3 V4L2 Display Driver > > Hi, > > On Wed, 2009-04-22 at 08:25 +0200, ext Hardik Shah wrote: > > This is the version 5th of the Driver. > > > > Following are the features supported. > > 1. Provides V4L2 user interface for the video pipelines of DSS > > 2. Basic streaming working on LCD and TV. > > 3. Support for various pixel formats like YUV, UYVY, RGB32, RGB24, RGB565 > > 4. Supports Alpha blending. > > 5. Supports Color keying both source and destination. > > 6. Supports rotation with RGB565 and RGB32 pixels formats. > > 7. Supports cropping. > > 8. Supports Background color setting. > > 9. Works on latest DSS2 library from Tomi > > http://www.bat.org/~tomba/git/linux-omap-dss.git/ > > 10. 1/4x scaling added. Detail testing left > > > > TODOS > > 1. Ioctls needs to be added for color space conversion matrix > > coefficient programming. > > 2. To be tested on DVI resolutions. > > > > Comments fixed from community. > > 1. V4L2 Driver for OMAP3/3 DSS. > > 2. Conversion of the custom ioctls to standard V4L2 ioctls like alpha > blending, > > color keying, rotation and back ground color setting > > 3. Re-organised the code as per community comments. > > 4. Added proper copyright year. > > 5. Added module name in printk > > 6. Kconfig option copy/paste error > > 7. Module param desc addded. > > 8. Query control implemented using standard query_fill > > 9. Re-arranged if-else constructs. > > 10. Changed to use mutex instead of semaphore. > > 11. Removed dual usage of rotation angles. > > 12. Implemented function to convert the V4L2 angle to DSS angle. > > 13. Y-position was set half by video driver for TV output > > Now its done by DSS so removed that. > > 14. Minor cleanup > > 15. Added support to pass the page offset to application. > > 14. Minor cleanup > > 15. Added support to pass the page offset to application. > > 16. Renamed V4L2_CID_ROTATION to V4L2_CID_ROTATE > > 17. Major comment from Hans fixed. > > 18. Copy right year changed. > > 19. Added module name for each error/warning print message. > > > > Changes from Previous Version. > > 1. Supported YUV rotation. > > 2. Supported Flipping. > > 3. Rebased line with Tomi's latest DSS2 master branch with commit id > > f575a02edf2218a18d6f2ced308b4f3e26b44ce2. > > 4. Kconfig option removed to select between the TV and LCD. > > Now supported dynamically by DSS2 library. > > 5. Kconfig option for the NTSC_M and PAL_BDGHI mode but not > > supported by DSS2. so it will not work now. > > There is basic support for this. See the DSS doc: > > /sys/devices/platform/omapdss/display? directory: > ... > timings Display timings (pixclock,xres/hfp/hbp/hsw,yres/vfp/vbp/vsw) > When writing, two special timings are accepted for tv-out: > "pal" and "ntsc" [Shah, Hardik] I was not aware of it will remove the compile time option and for now let the sysfs entry change the standard. In future I will try to do it with the S_STD and G_STD ioctls of the V4L2 framework. > > Tomi > > -- 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 3/3] OMAP2/3 V4L2 Display Driver
Hi, On Wed, 2009-04-22 at 08:25 +0200, ext Hardik Shah wrote: > This is the version 5th of the Driver. > > Following are the features supported. > 1. Provides V4L2 user interface for the video pipelines of DSS > 2. Basic streaming working on LCD and TV. > 3. Support for various pixel formats like YUV, UYVY, RGB32, RGB24, RGB565 > 4. Supports Alpha blending. > 5. Supports Color keying both source and destination. > 6. Supports rotation with RGB565 and RGB32 pixels formats. > 7. Supports cropping. > 8. Supports Background color setting. > 9. Works on latest DSS2 library from Tomi > http://www.bat.org/~tomba/git/linux-omap-dss.git/ > 10. 1/4x scaling added. Detail testing left > > TODOS > 1. Ioctls needs to be added for color space conversion matrix > coefficient programming. > 2. To be tested on DVI resolutions. > > Comments fixed from community. > 1. V4L2 Driver for OMAP3/3 DSS. > 2. Conversion of the custom ioctls to standard V4L2 ioctls like alpha > blending, > color keying, rotation and back ground color setting > 3. Re-organised the code as per community comments. > 4. Added proper copyright year. > 5. Added module name in printk > 6. Kconfig option copy/paste error > 7. Module param desc addded. > 8. Query control implemented using standard query_fill > 9. Re-arranged if-else constructs. > 10. Changed to use mutex instead of semaphore. > 11. Removed dual usage of rotation angles. > 12. Implemented function to convert the V4L2 angle to DSS angle. > 13. Y-position was set half by video driver for TV output > Now its done by DSS so removed that. > 14. Minor cleanup > 15. Added support to pass the page offset to application. > 14. Minor cleanup > 15. Added support to pass the page offset to application. > 16. Renamed V4L2_CID_ROTATION to V4L2_CID_ROTATE > 17. Major comment from Hans fixed. > 18. Copy right year changed. > 19. Added module name for each error/warning print message. > > Changes from Previous Version. > 1. Supported YUV rotation. > 2. Supported Flipping. > 3. Rebased line with Tomi's latest DSS2 master branch with commit id > f575a02edf2218a18d6f2ced308b4f3e26b44ce2. > 4. Kconfig option removed to select between the TV and LCD. > Now supported dynamically by DSS2 library. > 5. Kconfig option for the NTSC_M and PAL_BDGHI mode but not > supported by DSS2. so it will not work now. There is basic support for this. See the DSS doc: /sys/devices/platform/omapdss/display? directory: ... timings Display timings (pixclock,xres/hfp/hbp/hsw,yres/vfp/vbp/vsw) When writing, two special timings are accepted for tv-out: "pal" and "ntsc" Tomi -- 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: [Review PATCH 3/3] OMAP2/3 V4L2 Display Driver
Hi Hans,
My comments inlined.
Hardik,
> -Original Message-
> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> Sent: Tuesday, April 21, 2009 5:30 PM
> To: Shah, Hardik
> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav, Brijesh R;
> Hiremath, Vaibhav
> Subject: RE: [Review PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
> Hi Hardik,
>
> Just a few comments on your comments :-)
>
> > Hi Hans,
> > My Comments inlined,
> > Most of the comments are taken care off.
> > Thanks for review.
> >
> > Hardik,
> >
> >> -Original Message-
> >> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> >> Sent: Saturday, April 18, 2009 7:29 PM
> >> To: Shah, Hardik
> >> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav,
> >> Brijesh R;
> >> Hiremath, Vaibhav
> >> Subject: Re: [Review PATCH 3/3] OMAP2/3 V4L2 Display Driver
>
> >> > +config VID2_LCD_MANAGER
> >> > + bool "Use LCD Managaer"
> >> > + help
> >> > + Select this option if you want VID2 pipeline on LCD Overlay
> >> manager
> >> > +endchoice
> >> > +
> >> > +choice
> >> > +prompt "TV Mode"
> >> > +depends on VID2_TV_MANAGER || VID1_TV_MANAGER
> >> > +default NTSC_M
> >> > +
> >> > +config NTSC_M
> >> > +bool "Use NTSC_M mode"
> >> > +help
> >> > + Select this option if you want NTSC_M mode on TV
> >> > +
> >> > +config PAL_BDGHI
> >> > +bool "Use PAL_BDGHI mode"
> >> > +help
> >> > + Select this option if you want PAL_BDGHI mode on TV
> >>
> >> Terminology: PAL and NTSC etc. refer to broadcast standards. That is no
> >> generally applicable to omap. When it comes to streaming digital video
> >> there is only the 50 and 60 Hz SDTV standards. For output over a
> >> Composite
> >> or S-Video connector you can also choose between PAL and SECAM. There
> >> are some differences between the two, although I'm not sure about the
> >> details. The common saa7128 i2c device definitely has support for both.
> >>
> >> In this particular case you probably mean 50 or 60 Hz SDTV rather than
> >> NTSC/PAL.
> >>
> > [Shah, Hardik] OMAP DSS is having the internal video encoder for
> > converting the digital to analog standards like NTSC and PAL with a
> > s-video and composite output. Currently DSS does not support changing of
> > the TV standards dynamically so I have made it as a compile time option.
> > Once DSS will support that I will add the S_STD and G_STD for standards.
> > Internally it will call the DSS2 library APIs to change the standard.
>
> I don't have a problem with these config options, it's more the names
> NTSC_M and PAL_BDGHI that are misleading IMHO, since it's really a matter
> of 50Hz vs 60Hz and not of NTSC vs PAL.
[Shah, Hardik] Video Encoder of the DSS2 supports number of PAL and NTSC
standards out of them only two are supported to I just tried to be specific as
50Hz may mean any PAL standards other than PAL_BDGHI and 60 may mean any NTSC
standard other than NTSC_M. Any way this is not going to be permanent.
>
> >> > + if (1 == vout->mirror && vout->rotation >= 0) {
> >> > + rotation_deg = (vout->rotation == 1) ?
> >> > + 3 : (vout->rotation == 3) ?
> >> > + 1 : (vout->rotation == 2) ?
> >> > + 0 : 2;
> >>
> >> Or: rotation = (4 - vout->rotation) % 4;
> > [Shah, Hardik] It will not work when rotation will be 0 and I want 2
> > because of mirroring. (4-0) % 2 is 0 I want 2 here. So not changing it.
>
> You are right. Sorry about that.
>
> >> > +static int vidioc_s_ctrl(struct file *file, void *fh, struct
> >> v4l2_control
> >> *a)
> >> > +{
> >> > + struct omap_vout_device *vout = ((struct omap_vout_fh *)
> >> fh)->vout;
> >> > +
> >> > + switch (a->id) {
> >> > + case V4L2_CID_ROTATE:
> >> > + {
> >> > + int rotation = a->value;
> >> > +
> >> > + if (vout->pix.pixelformat == V4L2_PIX_FMT_RGB24 &&
> >> > + rotation != -1)
> >>
> >> Huh? Shouldn't this be rotation != 0?
> > [Shah, Hardik] Rotation 0 means the rotation using Virtual Frame Buffer
> > Rotation (VRFB) engine. It does not support rotation with packed RGB24
> > format. -1 means VRFB is not used. So it should be -1;
>
> This really isn't right. a->value is the rotate control value. And that's
> defined as 0, 90, 180 or 270 according to queryctrl. Not -1. The value -1
> is a purely internal value which is also why I am opposed to its use. It
> is very confusing.
[Shah, Hardik] Agreed,
I will change that.
>
> Regards,
>
> Hans
>
> --
> Hans Verkuil - video4linux developer - sponsored by TANDBERG
>
--
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: [Review PATCH 3/3] OMAP2/3 V4L2 Display Driver
Hi Hardik,
Just a few comments on your comments :-)
> Hi Hans,
> My Comments inlined,
> Most of the comments are taken care off.
> Thanks for review.
>
> Hardik,
>
>> -Original Message-
>> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
>> Sent: Saturday, April 18, 2009 7:29 PM
>> To: Shah, Hardik
>> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Jadav,
>> Brijesh R;
>> Hiremath, Vaibhav
>> Subject: Re: [Review PATCH 3/3] OMAP2/3 V4L2 Display Driver
>> > +config VID2_LCD_MANAGER
>> > + bool "Use LCD Managaer"
>> > + help
>> > + Select this option if you want VID2 pipeline on LCD Overlay
>> manager
>> > +endchoice
>> > +
>> > +choice
>> > +prompt "TV Mode"
>> > +depends on VID2_TV_MANAGER || VID1_TV_MANAGER
>> > +default NTSC_M
>> > +
>> > +config NTSC_M
>> > +bool "Use NTSC_M mode"
>> > +help
>> > + Select this option if you want NTSC_M mode on TV
>> > +
>> > +config PAL_BDGHI
>> > +bool "Use PAL_BDGHI mode"
>> > +help
>> > + Select this option if you want PAL_BDGHI mode on TV
>>
>> Terminology: PAL and NTSC etc. refer to broadcast standards. That is no
>> generally applicable to omap. When it comes to streaming digital video
>> there is only the 50 and 60 Hz SDTV standards. For output over a
>> Composite
>> or S-Video connector you can also choose between PAL and SECAM. There
>> are some differences between the two, although I'm not sure about the
>> details. The common saa7128 i2c device definitely has support for both.
>>
>> In this particular case you probably mean 50 or 60 Hz SDTV rather than
>> NTSC/PAL.
>>
> [Shah, Hardik] OMAP DSS is having the internal video encoder for
> converting the digital to analog standards like NTSC and PAL with a
> s-video and composite output. Currently DSS does not support changing of
> the TV standards dynamically so I have made it as a compile time option.
> Once DSS will support that I will add the S_STD and G_STD for standards.
> Internally it will call the DSS2 library APIs to change the standard.
I don't have a problem with these config options, it's more the names
NTSC_M and PAL_BDGHI that are misleading IMHO, since it's really a matter
of 50Hz vs 60Hz and not of NTSC vs PAL.
>> > + if (1 == vout->mirror && vout->rotation >= 0) {
>> > + rotation_deg = (vout->rotation == 1) ?
>> > + 3 : (vout->rotation == 3) ?
>> > + 1 : (vout->rotation == 2) ?
>> > + 0 : 2;
>>
>> Or: rotation = (4 - vout->rotation) % 4;
> [Shah, Hardik] It will not work when rotation will be 0 and I want 2
> because of mirroring. (4-0) % 2 is 0 I want 2 here. So not changing it.
You are right. Sorry about that.
>> > +static int vidioc_s_ctrl(struct file *file, void *fh, struct
>> v4l2_control
>> *a)
>> > +{
>> > + struct omap_vout_device *vout = ((struct omap_vout_fh *)
>> fh)->vout;
>> > +
>> > + switch (a->id) {
>> > + case V4L2_CID_ROTATE:
>> > + {
>> > + int rotation = a->value;
>> > +
>> > + if (vout->pix.pixelformat == V4L2_PIX_FMT_RGB24 &&
>> > + rotation != -1)
>>
>> Huh? Shouldn't this be rotation != 0?
> [Shah, Hardik] Rotation 0 means the rotation using Virtual Frame Buffer
> Rotation (VRFB) engine. It does not support rotation with packed RGB24
> format. -1 means VRFB is not used. So it should be -1;
This really isn't right. a->value is the rotate control value. And that's
defined as 0, 90, 180 or 270 according to queryctrl. Not -1. The value -1
is a purely internal value which is also why I am opposed to its use. It
is very confusing.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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
