mbox

[GIT,PULL,for,v3.0] OMAP_VOUT bug fixes and code cleanup

Message ID 1308771169-10741-1-git-send-email-hvaibhav@ti.com (mailing list archive)
State Superseded, archived
Headers

Pull-request

git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-linux-media

Message

Hiremath, Vaibhav June 22, 2011, 7:32 p.m. UTC 
  The following changes since commit af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
  Linus Torvalds (1):
        Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip

are available in the git repository at:

  git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-linux-media

Amber Jain (2):
      V4L2: omap_vout: Remove GFP_DMA allocation as ZONE_DMA is not configured on OMAP
      OMAP2: V4L2: Remove GFP_DMA allocation as ZONE_DMA is not configured on OMAP

Archit Taneja (3):
      OMAP_VOUT: CLEANUP: Move generic functions and macros to common files
      OMAP_VOUT: CLEANUP: Make rotation related helper functions more descriptive
      OMAP_VOUT: Create separate file for VRFB related API's

Vaibhav Hiremath (2):
      OMAP_VOUT: Change hardcoded device node number to -1
      omap_vout: Added check in reqbuf & mmap for buf_size allocation

Vladimir Pantelic (1):
      OMAP_VOUTLIB: Fix wrong resizer calculation

 drivers/media/video/omap/Kconfig          |    7 +-
 drivers/media/video/omap/Makefile         |    1 +
 drivers/media/video/omap/omap_vout.c      |  602 ++++++-----------------------
 drivers/media/video/omap/omap_vout_vrfb.c |  390 +++++++++++++++++++
 drivers/media/video/omap/omap_vout_vrfb.h |   40 ++
 drivers/media/video/omap/omap_voutdef.h   |   78 ++++
 drivers/media/video/omap/omap_voutlib.c   |   52 +++-
 drivers/media/video/omap/omap_voutlib.h   |   12 +-
 drivers/media/video/omap24xxcam.c         |    4 +-
 9 files changed, 684 insertions(+), 502 deletions(-)
 create mode 100644 drivers/media/video/omap/omap_vout_vrfb.c
 create mode 100644 drivers/imedia/video/omap/omap_vout_vrfb.h


 These patches include bug fixes and code cleanup.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mauro Carvalho Chehab July 1, 2011, 6:43 p.m. UTC | #1 
Em 22-06-2011 16:32, hvaibhav@ti.com escreveu:
> The following changes since commit af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
>   Linus Torvalds (1):
>         Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip
> 
> are available in the git repository at:
> 
>   git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-linux-media
> 
> Amber Jain (2):
>       V4L2: omap_vout: Remove GFP_DMA allocation as ZONE_DMA is not configured on OMAP
>       OMAP2: V4L2: Remove GFP_DMA allocation as ZONE_DMA is not configured on OMAP

> From: Amber Jain <amber@ti.com>
> Date: Tue May 31 11:45:36 2011 -0300
> 
> OMAP2: V4L2: Remove GFP_DMA allocation as ZONE_DMA is not configured on OMAP
>     
> Remove GFP_DMA from the __get_free_pages() call from omap24xxcam as ZONE_DMA
> is not configured on OMAP. Earlier the page allocator used to return a page
> from ZONE_NORMAL even when GFP_DMA is passed and CONFIG_ZONE_DMA is disabled.
> As a result of commit a197b59ae6e8bee56fcef37ea2482dc08414e2ac, page allocator
> returns null in such a scenario with a warning emitted to kernel log.
>    
> Signed-off-by: Amber Jain <amber@ti.com>
> Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> 
> diff --git a/drivers/media/video/omap24xxcam.c b/drivers/media/video/omap24xxcam.c
> index f6626e8..d92d4c6 100644
> --- a/drivers/media/video/omap24xxcam.c
> +++ b/drivers/media/video/omap24xxcam.c
> @@ -309,11 +309,11 @@ static int omap24xxcam_vbq_alloc_mmap_buffer(struct videobuf_buffer *vb)
>  			order--;
>  
>  		/* try to allocate as many contiguous pages as possible */
> -		page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> +		page = alloc_pages(GFP_KERNEL, order);
>  		/* if allocation fails, try to allocate smaller amount */
>  		while (page == NULL) {
>  			order--;
> -			page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> +			page = alloc_pages(GFP_KERNEL, order);
>  			if (page == NULL && !order) {
>  				err = -ENOMEM;
>  				goto out;

Hmm... the proper fix wouldn't be to define ZONE_DMA at OMAP?

Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab July 1, 2011, 6:48 p.m. UTC | #2 
Em 22-06-2011 16:32, hvaibhav@ti.com escreveu:
> The following changes since commit af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
>   Linus Torvalds (1):
>         Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip
> 
> are available in the git repository at:
> 
>   git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-linux-media
> 

> Archit Taneja (3):
>       OMAP_VOUT: CLEANUP: Move generic functions and macros to common files
>       OMAP_VOUT: CLEANUP: Make rotation related helper functions more descriptive
>       OMAP_VOUT: Create separate file for VRFB related API's

Those are cleanup patches. NACK for 3.0. Cleanups should be sent to -next kernel (3.1).

> Vaibhav Hiremath (2):
>       OMAP_VOUT: Change hardcoded device node number to -1
>       omap_vout: Added check in reqbuf & mmap for buf_size allocation
> 
> Vladimir Pantelic (1):
>       OMAP_VOUTLIB: Fix wrong resizer calculation

The 3 above patches are ok for 3.0. Added.

Thanks,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Archit Taneja July 2, 2011, 1:31 a.m. UTC | #3 
Hi,

On Friday 01 July 2011 04:44 PM, Mauro Carvalho Chehab wrote:
> Em 01-07-2011 19:49, Archit Taneja escreveu:
>> Hi,
>>
>> On Friday 01 July 2011 11:48 AM, Mauro Carvalho Chehab wrote:
>>> Em 22-06-2011 16:32, hvaibhav@ti.com escreveu:
>>>> The following changes since commit af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
>>>>     Linus Torvalds (1):
>>>>           Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip
>>>>
>>>> are available in the git repository at:
>>>>
>>>>     git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-linux-media
>>>>
>>>
>>>> Archit Taneja (3):
>>>>         OMAP_VOUT: CLEANUP: Move generic functions and macros to common files
>>>>         OMAP_VOUT: CLEANUP: Make rotation related helper functions more descriptive
>>>>         OMAP_VOUT: Create separate file for VRFB related API's
>>>
>>> Those are cleanup patches. NACK for 3.0. Cleanups should be sent to -next kernel (3.1).
>>
>> The first 2 patches are pre-requisite cleanup patches required for the 3rd patch. The third patch adds functionality, it prevents the driver using VRFB for OMAP4, and forces the use of SDMA buffers.
>>
>> So the patch set as a whole is not only cleanup, and I guess we could push it for 3.0.
>
> Patches that add new functionalities shold also be delayed to -next. Only bug fixes
> are allowed during the -rc cycle. Even patch 3 were a fix, you would need to rebase
> it to not depend on the cleanups, for it to be accepted on a -rc kernel.

Okay, I get it now. I'll ask Vaibhav to queue this for the -next kernel.

Thanks,
Archit
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiremath, Vaibhav July 4, 2011, 5:09 a.m. UTC | #4 
> -----Original Message-----
> From: Mauro Carvalho Chehab [mailto:mchehab@redhat.com]
> Sent: Saturday, July 02, 2011 12:19 AM
> To: Hiremath, Vaibhav
> Cc: linux-media@vger.kernel.org
> Subject: Re: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> 
> Em 22-06-2011 16:32, hvaibhav@ti.com escreveu:
> > The following changes since commit
> af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
> >   Linus Torvalds (1):
> >         Merge branch 'x86-urgent-for-linus' of
> git://git.kernel.org/.../tip/linux-2.6-tip
> >
> > are available in the git repository at:
> >
> >   git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-
> linux-media
> >
> 
> > Archit Taneja (3):
> >       OMAP_VOUT: CLEANUP: Move generic functions and macros to common
> files
> >       OMAP_VOUT: CLEANUP: Make rotation related helper functions more
> descriptive
> >       OMAP_VOUT: Create separate file for VRFB related API's
> 
> Those are cleanup patches. NACK for 3.0. Cleanups should be sent to -next
> kernel (3.1).
> 
[Hiremath, Vaibhav] Ok, will queue it for 3.1.


> > Vaibhav Hiremath (2):
> >       OMAP_VOUT: Change hardcoded device node number to -1
> >       omap_vout: Added check in reqbuf & mmap for buf_size allocation
> >
> > Vladimir Pantelic (1):
> >       OMAP_VOUTLIB: Fix wrong resizer calculation
> 
> The 3 above patches are ok for 3.0. Added.
> 
[Hiremath, Vaibhav] Thanks,

Thanks,
Vaibhav

> Thanks,
> Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amber Jain July 5, 2011, 11:34 a.m. UTC | #5 
> -----Original Message-----
> From: Mauro Carvalho Chehab [mailto:mchehab@redhat.com]
> Sent: Saturday, July 02, 2011 12:13 AM
> To: Hiremath, Vaibhav
> Cc: linux-media@vger.kernel.org; JAIN, AMBER; David Rientjes; Andrew
> Morton
> Subject: Re: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> 
> Em 22-06-2011 16:32, hvaibhav@ti.com escreveu:
> > The following changes since commit
> af0d6a0a3a30946f7df69c764791f1b0643f7cd6:
> >   Linus Torvalds (1):
> >         Merge branch 'x86-urgent-for-linus' of
> git://git.kernel.org/.../tip/linux-2.6-tip
> >
> > are available in the git repository at:
> >
> >   git://arago-project.org/git/people/vaibhav/ti-psp-omap-video.git for-
> linux-media
> >
> > Amber Jain (2):
> >       V4L2: omap_vout: Remove GFP_DMA allocation as ZONE_DMA is not
> configured on OMAP
> >       OMAP2: V4L2: Remove GFP_DMA allocation as ZONE_DMA is not
> configured on OMAP
> 
> > From: Amber Jain <amber@ti.com>
> > Date: Tue May 31 11:45:36 2011 -0300
> >
> > OMAP2: V4L2: Remove GFP_DMA allocation as ZONE_DMA is not configured on
> OMAP
> >
> > Remove GFP_DMA from the __get_free_pages() call from omap24xxcam as
> ZONE_DMA
> > is not configured on OMAP. Earlier the page allocator used to return a
> page
> > from ZONE_NORMAL even when GFP_DMA is passed and CONFIG_ZONE_DMA is
> disabled.
> > As a result of commit a197b59ae6e8bee56fcef37ea2482dc08414e2ac, page
> allocator
> > returns null in such a scenario with a warning emitted to kernel log.
> >
> > Signed-off-by: Amber Jain <amber@ti.com>
> > Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> >
> > diff --git a/drivers/media/video/omap24xxcam.c
> b/drivers/media/video/omap24xxcam.c
> > index f6626e8..d92d4c6 100644
> > --- a/drivers/media/video/omap24xxcam.c
> > +++ b/drivers/media/video/omap24xxcam.c
> > @@ -309,11 +309,11 @@ static int
> omap24xxcam_vbq_alloc_mmap_buffer(struct videobuf_buffer *vb)
> >  			order--;
> >
> >  		/* try to allocate as many contiguous pages as possible */
> > -		page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > +		page = alloc_pages(GFP_KERNEL, order);
> >  		/* if allocation fails, try to allocate smaller amount */
> >  		while (page == NULL) {
> >  			order--;
> > -			page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > +			page = alloc_pages(GFP_KERNEL, order);
> >  			if (page == NULL && !order) {
> >  				err = -ENOMEM;
> >  				goto out;
> 
> Hmm... the proper fix wouldn't be to define ZONE_DMA at OMAP?

I don't think so, my understanding for ZOME_DMA is that it is defined for architectures that have restrictions on memory addresses that can be used for DMA. OMAP doesn't have any such restriction and hence we should not define ZONE_DMA.

Please let me know if I have missed some point.

Thanks,
Amber
> 
> Thanks,
> Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Rientjes July 6, 2011, 8:43 p.m. UTC | #6 
On Tue, 5 Jul 2011, JAIN, AMBER wrote:

> > > diff --git a/drivers/media/video/omap24xxcam.c
> > b/drivers/media/video/omap24xxcam.c
> > > index f6626e8..d92d4c6 100644
> > > --- a/drivers/media/video/omap24xxcam.c
> > > +++ b/drivers/media/video/omap24xxcam.c
> > > @@ -309,11 +309,11 @@ static int
> > omap24xxcam_vbq_alloc_mmap_buffer(struct videobuf_buffer *vb)
> > >  			order--;
> > >
> > >  		/* try to allocate as many contiguous pages as possible */
> > > -		page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > +		page = alloc_pages(GFP_KERNEL, order);
> > >  		/* if allocation fails, try to allocate smaller amount */
> > >  		while (page == NULL) {
> > >  			order--;
> > > -			page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > +			page = alloc_pages(GFP_KERNEL, order);
> > >  			if (page == NULL && !order) {
> > >  				err = -ENOMEM;
> > >  				goto out;
> > 
> > Hmm... the proper fix wouldn't be to define ZONE_DMA at OMAP?
> 
> I don't think so, my understanding for ZOME_DMA is that it is defined 
> for architectures that have restrictions on memory addresses that can be 
> used for DMA. OMAP doesn't have any such restriction and hence we should 
> not define ZONE_DMA.
> 

s/should not define/do not need to define/

Right, if omap does not have DMA restrictions then the GFP_DMA usage that 
is removed with this patch was incorrect.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiremath, Vaibhav July 7, 2011, 5:55 a.m. UTC | #7 
> -----Original Message-----
> From: David Rientjes [mailto:rientjes@google.com]
> Sent: Thursday, July 07, 2011 2:14 AM
> To: JAIN, AMBER
> Cc: Mauro Carvalho Chehab; Hiremath, Vaibhav; linux-media@vger.kernel.org;
> Andrew Morton
> Subject: RE: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> 
> On Tue, 5 Jul 2011, JAIN, AMBER wrote:
> 
> > > > diff --git a/drivers/media/video/omap24xxcam.c
> > > b/drivers/media/video/omap24xxcam.c
> > > > index f6626e8..d92d4c6 100644
> > > > --- a/drivers/media/video/omap24xxcam.c
> > > > +++ b/drivers/media/video/omap24xxcam.c
> > > > @@ -309,11 +309,11 @@ static int
> > > omap24xxcam_vbq_alloc_mmap_buffer(struct videobuf_buffer *vb)
> > > >  			order--;
> > > >
> > > >  		/* try to allocate as many contiguous pages as possible
> */
> > > > -		page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > > +		page = alloc_pages(GFP_KERNEL, order);
> > > >  		/* if allocation fails, try to allocate smaller amount
> */
> > > >  		while (page == NULL) {
> > > >  			order--;
> > > > -			page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > > +			page = alloc_pages(GFP_KERNEL, order);
> > > >  			if (page == NULL && !order) {
> > > >  				err = -ENOMEM;
> > > >  				goto out;
> > >
> > > Hmm... the proper fix wouldn't be to define ZONE_DMA at OMAP?
> >
> > I don't think so, my understanding for ZOME_DMA is that it is defined
> > for architectures that have restrictions on memory addresses that can be
> > used for DMA. OMAP doesn't have any such restriction and hence we should
> > not define ZONE_DMA.
> >
> 
> s/should not define/do not need to define/
> 
> Right, if omap does not have DMA restrictions then the GFP_DMA usage that
> is removed with this patch was incorrect.
[Hiremath, Vaibhav] I did not understand your comment; can you please help me to understand here?

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amber Jain July 7, 2011, 5:57 a.m. UTC | #8 
> -----Original Message-----
> From: Hiremath, Vaibhav
> Sent: Thursday, July 07, 2011 11:25 AM
> To: David Rientjes; JAIN, AMBER
> Cc: Mauro Carvalho Chehab; linux-media@vger.kernel.org; Andrew Morton
> Subject: RE: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> 
> > -----Original Message-----
> > From: David Rientjes [mailto:rientjes@google.com]
> > Sent: Thursday, July 07, 2011 2:14 AM
> > To: JAIN, AMBER
> > Cc: Mauro Carvalho Chehab; Hiremath, Vaibhav; linux-
> media@vger.kernel.org;
> > Andrew Morton
> > Subject: RE: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> >
> > On Tue, 5 Jul 2011, JAIN, AMBER wrote:
> >
> > > > > diff --git a/drivers/media/video/omap24xxcam.c
> > > > b/drivers/media/video/omap24xxcam.c
> > > > > index f6626e8..d92d4c6 100644
> > > > > --- a/drivers/media/video/omap24xxcam.c
> > > > > +++ b/drivers/media/video/omap24xxcam.c
> > > > > @@ -309,11 +309,11 @@ static int
> > > > omap24xxcam_vbq_alloc_mmap_buffer(struct videobuf_buffer *vb)
> > > > >  			order--;
> > > > >
> > > > >  		/* try to allocate as many contiguous pages as possible
> > */
> > > > > -		page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > > > +		page = alloc_pages(GFP_KERNEL, order);
> > > > >  		/* if allocation fails, try to allocate smaller amount
> > */
> > > > >  		while (page == NULL) {
> > > > >  			order--;
> > > > > -			page = alloc_pages(GFP_KERNEL | GFP_DMA, order);
> > > > > +			page = alloc_pages(GFP_KERNEL, order);
> > > > >  			if (page == NULL && !order) {
> > > > >  				err = -ENOMEM;
> > > > >  				goto out;
> > > >
> > > > Hmm... the proper fix wouldn't be to define ZONE_DMA at OMAP?
> > >
> > > I don't think so, my understanding for ZOME_DMA is that it is defined
> > > for architectures that have restrictions on memory addresses that can
> be
> > > used for DMA. OMAP doesn't have any such restriction and hence we
> should
> > > not define ZONE_DMA.
> > >
> >
> > s/should not define/do not need to define/
> >
> > Right, if omap does not have DMA restrictions then the GFP_DMA usage
> that
> > is removed with this patch was incorrect.
> [Hiremath, Vaibhav] I did not understand your comment; can you please help
> me to understand here?

I think what David wants to say is that if we do not have DMA restrictions on OMAP we should have not used GFP_DMA flag for page allocation at first place. Is my understanding correct David?

~Amber

> 
> Thanks,
> Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Rientjes July 8, 2011, 8:16 p.m. UTC | #9 
On Thu, 7 Jul 2011, JAIN, AMBER wrote:

> I think what David wants to say is that if we do not have DMA 
> restrictions on OMAP we should have not used GFP_DMA flag for page 
> allocation at first place. Is my understanding correct David?
> 

Right, and that's what the patch is doing.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiremath, Vaibhav July 11, 2011, 1:35 p.m. UTC | #10 
> -----Original Message-----
> From: David Rientjes [mailto:rientjes@google.com]
> Sent: Saturday, July 09, 2011 1:47 AM
> To: JAIN, AMBER
> Cc: Hiremath, Vaibhav; Mauro Carvalho Chehab; linux-media@vger.kernel.org;
> Andrew Morton
> Subject: RE: [GIT PULL for v3.0] OMAP_VOUT bug fixes and code cleanup
> 
> On Thu, 7 Jul 2011, JAIN, AMBER wrote:
> 
> > I think what David wants to say is that if we do not have DMA
> > restrictions on OMAP we should have not used GFP_DMA flag for page
> > allocation at first place. Is my understanding correct David?
> >
> 
> Right, and that's what the patch is doing.
[Hiremath, Vaibhav] I do not remember now how that flag got added. However, now I would say that it was a miss; or rather, it could be "copy-paste mistake".

Thanks,
Vaibhav

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html