[PULL for v3.5] SH Mobile LCDC overlay support

[PULL for v3.5] SH Mobile LCDC overlay support

[Laurent Pinchart]

Hi Florian,

Could you please pull the following patches that add overlay support for the 
SH Mobile LCDC ? They have been posted for review as part of a bigger series 
some time ago.

The following changes since commit d313a86d2efb2c5568832389663322e93e291c59:

  i.MX28: Shut down the LCD controller to avoid BootROM sampling bug 
(2012-04-21 21:53:12 +0000)

are available in the git repository at:
  git://linuxtv.org/pinchartl/fbdev.git planes

Laurent Pinchart (3):
      fbdev: sh_mobile_lcdc: Constify sh_mobile_lcdc_fix structure
      fbdev: sh_mobile_lcdc: Rename fb operation handlers with a common prefix
      fbdev: sh_mobile_lcdc: Implement overlays support

 drivers/video/sh_mobile_lcdcfb.c | 1004 +++++++++++++++++++++++++++++++++----
 include/video/sh_mobile_lcdc.h   |    7 +
 2 files changed, 915 insertions(+), 96 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PULL for v3.5] SH Mobile LCDC overlay support

[Florian Tobias Schandinat]

Hi Laurent,

On 04/24/2012 11:31 AM, Laurent Pinchart wrote:
> Hi Florian,
> 
> Could you please pull the following patches that add overlay support for the 
> SH Mobile LCDC ? They have been posted for review as part of a bigger series 
> some time ago.

Well, besides that I dislike everyone inventing his/her own overlay API
and just not having time to propose something that fits everyone your
patch looks mostly okay. Two things:

"That's fine, but you need a Documentation/ABI entry for any new sysfs
file you create." [Greg K-H]
So as far as I understand Gregs statement and your code correct you
should add such thing.

I'm not convinced that you don't leak resources. Is the memory you
allocate with dma_alloc_coherent for the overlays really freed after
normal use and in error paths?


Best regards,

Florian Tobias Schandinat

> 
> The following changes since commit d313a86d2efb2c5568832389663322e93e291c59:
> 
>   i.MX28: Shut down the LCD controller to avoid BootROM sampling bug 
> (2012-04-21 21:53:12 +0000)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/fbdev.git planes
> 
> Laurent Pinchart (3):
>       fbdev: sh_mobile_lcdc: Constify sh_mobile_lcdc_fix structure
>       fbdev: sh_mobile_lcdc: Rename fb operation handlers with a common prefix
>       fbdev: sh_mobile_lcdc: Implement overlays support
> 
>  drivers/video/sh_mobile_lcdcfb.c | 1004 +++++++++++++++++++++++++++++++++----
>  include/video/sh_mobile_lcdc.h   |    7 +
>  2 files changed, 915 insertions(+), 96 deletions(-)
> 

Re: [PULL for v3.5] SH Mobile LCDC overlay support

[Laurent Pinchart]

Hi Florian,

On Sunday 13 May 2012 20:36:29 Florian Tobias Schandinat wrote:
> On 04/24/2012 11:31 AM, Laurent Pinchart wrote:
> > Hi Florian,
> > 
> > Could you please pull the following patches that add overlay support for
> > the SH Mobile LCDC ? They have been posted for review as part of a bigger
> > series some time ago.
> 
> Well, besides that I dislike everyone inventing his/her own overlay API
> and just not having time to propose something that fits everyone your
> patch looks mostly okay.

This is a topic that we started to discuss at the ELC a couple of months ago. 
My goal was to come up with a single API shared between DRM/KMS, FB and V4L2 
to handle graphics pipeline configuration. It quickly turned out that there 
was not enough shared interest between DRM and V4L2 developers to make this 
happen. Should we resume the discussion for FB, or would you rather want a 
separate FB-specific API ?

> Two things:
> 
> "That's fine, but you need a Documentation/ABI entry for any new sysfs
> file you create." [Greg K-H]

Thank you for reminding me.

> So as far as I understand Gregs statement and your code correct you
> should add such thing.
> 
> I'm not convinced that you don't leak resources. Is the memory you
> allocate with dma_alloc_coherent for the overlays really freed after
> normal use and in error paths?

I'm not sure where the cleanup part of the patch went to, but definitely not 
in that branch :-/ I'll fix that and resubmit.

-- 
Best regards,

Laurent Pinchart