Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 0/2] viafb modetable conversion
From: Florian Tobias Schandinat @ 2012-03-08 17:55 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-kernel, Florian Tobias Schandinat

Hi all,

finally the modetable conversion is here. The first patch does the 
conversion to get rid of some strange things and lot of defines.
The conversion was done by a program and causes a lot of warnings by 
generating lines longer than 80 characters which I do not consider 
worth fixing. A side effect of this conversion seems to be that CRTs 
are happier with the resulting modes/clocks as the conversion program 
used floating point and therefore the resulting pixclocks are closer to 
what they should be.
The second patch is just sort of performance optimization by avoiding 
two unnecessary modetable walks.


Best regards,

Florian Tobias Schandinat


Florian Tobias Schandinat (2):
  viafb: modetable conversion
  viafb: avoid refresh and mode lookup in set_par

 drivers/video/via/dvi.c      |    2 +-
 drivers/video/via/hw.c       |   41 +--
 drivers/video/via/hw.h       |    4 +-
 drivers/video/via/lcd.c      |   53 +---
 drivers/video/via/share.h    |  331 --------------------
 drivers/video/via/viafbdev.c |   32 +-
 drivers/video/via/viamode.c  |  713 ++++++------------------------------------
 drivers/video/via/viamode.h  |   11 +-
 8 files changed, 134 insertions(+), 1053 deletions(-)

-- 
1.7.9


^ permalink raw reply

* Re: [PATCH] drm: exynos: Fix fb_videomode <-> drm_mode_modeinfo conversion
From: James Simmons @ 2012-03-08 16:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev, Seung-Woo Kim, dri-devel, Inki Dae, Kyungmin Park
In-Reply-To: <1331206495-3853-1-git-send-email-laurent.pinchart@ideasonboard.com>


> The fb_videomode structure stores the front porch and back porch in the
> right_margin and left_margin fields respectively. right_margin should
> thus be computed with hsync_start - hdisplay, and left_margin with
> htotal - hsync_end. The same holds for the vertical direction.
> 
>        Active               Front           Sync            Back
>        Region               Porch                           Porch
> <-------------------><----------------><-------------><---------------->
> 
>   //////////////////|
>  ////////////////// |
> //////////////////  |..................               ..................
>                                        _______________
> 
> <------ xres -------><- right_margin -><- hsync_len -><- left_margin -->
> 
> <---- hdisplay ----->
> <------------ hsync_start ------------>
> <--------------------- hsync_end -------------------->
> <--------------------------------- htotal ----------------------------->
> 
> Fix the fb_videomode <-> drm_mode_modeinfo conversion functions
> accordingly.
> 

Wow I see this has remegered. Some time last year I posted a patch that 
had these routines in a generic format for people to use. I can repost 
them again if people are interested.


^ permalink raw reply

* Re: [RFC] FB_MODE_IS_* usage
From: James Simmons @ 2012-03-08 16:55 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <2317885.WEEKmd8lWO@avalon>

> Hi everybody,
> 
> While going through users of the fb_videomode structure, I realized that some 
> of the FB_MODE_IS_* flags didn't seem to be used for any practical purpose.
> 
> The flags are defined as
> 
> #define FB_MODE_IS_UNKNOWN      0
> #define FB_MODE_IS_DETAILED     1
> #define FB_MODE_IS_STANDARD     2
> #define FB_MODE_IS_VESA         4
> #define FB_MODE_IS_CALCULATED   8
> #define FB_MODE_IS_FIRST        16
> #define FB_MODE_IS_FROM_VAR     32
> 
> - FB_MODE_IS_UNKNOWN isn't used at all

This was for weird situations. 
 
> - FB_MODE_IS_DETAILED is set by the EDID parser for modes coming from detailed 
> timing descriptors, and checked right after to work around monitors that 
> report the "Preferred timing mode specified in descriptor block 1" bit in 
> their EDID block, but don't report any detailed timing descriptor. 
> FB_MODE_IS_DETAILED is thus only used internally in the EDID parser.
> 
> - FB_MODE_IS_STANDARD, FB_MODE_IS_VESA and FB_MODE_IS_CALCULATED are set but 
> never used.

The reasoning behind this was struct fb_info contains a struct fb_monspecs 
which had a private database of modes. So when the driver would unload 
that it would skip deleting this entries. That way if drivers needed 
special modes besides the standard modes it db could contain them.

> - FB_MODE_IS_FIRST is set by the EDID parser on the first detailed mode, and 
> checked by fb_find_best_display() and a couple of drivers (fsl-diu-fb, 
> sh_mobile_hdmi, aty and riva) to locate the first detailed mode (I will check 
> if I can modify those 4 drivers to use fb_find_best_display() or something 
> similar).

Okay.

> - FB_MODE_IS_FROM_VAR is set but never used.

Again in the case of using the private db the driver would mark that mode
with this flag so it would be aware that it is safe to delete.

> If I can remove direct use of the FB_MODE_IS_FIRST flag in the drivers 
> mentioned above, the FB_MODE_IS_DETAILED and FB_MODE_IS_FIRST flags will only 
> be used internally in the EDID parser. All the other flags are either not used 
> at all, or set but never read.
> 
> I'm working on sharing the EDID parser between DRM/KMS, FB and V4L2. The 
> FB_MODE_IS_FIRST and FB_MODE_IS_DETAILED flags will thus become unused. Is 
> there any objection to removing them then ?

Yah !!!


^ permalink raw reply

* [RFC] FB_MODE_IS_* usage
From: Laurent Pinchart @ 2012-03-08 16:26 UTC (permalink / raw)
  To: linux-fbdev

Hi everybody,

While going through users of the fb_videomode structure, I realized that some 
of the FB_MODE_IS_* flags didn't seem to be used for any practical purpose.

The flags are defined as

#define FB_MODE_IS_UNKNOWN      0
#define FB_MODE_IS_DETAILED     1
#define FB_MODE_IS_STANDARD     2
#define FB_MODE_IS_VESA         4
#define FB_MODE_IS_CALCULATED   8
#define FB_MODE_IS_FIRST        16
#define FB_MODE_IS_FROM_VAR     32

- FB_MODE_IS_UNKNOWN isn't used at all

- FB_MODE_IS_DETAILED is set by the EDID parser for modes coming from detailed 
timing descriptors, and checked right after to work around monitors that 
report the "Preferred timing mode specified in descriptor block 1" bit in 
their EDID block, but don't report any detailed timing descriptor. 
FB_MODE_IS_DETAILED is thus only used internally in the EDID parser.

- FB_MODE_IS_STANDARD, FB_MODE_IS_VESA and FB_MODE_IS_CALCULATED are set but 
never used.

- FB_MODE_IS_FIRST is set by the EDID parser on the first detailed mode, and 
checked by fb_find_best_display() and a couple of drivers (fsl-diu-fb, 
sh_mobile_hdmi, aty and riva) to locate the first detailed mode (I will check 
if I can modify those 4 drivers to use fb_find_best_display() or something 
similar).

- FB_MODE_IS_FROM_VAR is set but never used.

If I can remove direct use of the FB_MODE_IS_FIRST flag in the drivers 
mentioned above, the FB_MODE_IS_DETAILED and FB_MODE_IS_FIRST flags will only 
be used internally in the EDID parser. All the other flags are either not used 
at all, or set but never read.

I'm working on sharing the EDID parser between DRM/KMS, FB and V4L2. The 
FB_MODE_IS_FIRST and FB_MODE_IS_DETAILED flags will thus become unused. Is 
there any objection to removing them then ?

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] drm: exynos: Fix fb_videomode <-> drm_mode_modeinfo conversion
From: Sascha Hauer @ 2012-03-08 15:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-fbdev, Seung-Woo Kim, dri-devel, Inki Dae, Kyungmin Park
In-Reply-To: <2708446.uONhtK6raB@avalon>

On Thu, Mar 08, 2012 at 04:31:51PM +0100, Laurent Pinchart wrote:
> Hi Joonyoung,
> 
> On Thursday 08 March 2012 22:05:50 Joonyoung Shim wrote:
> > On 03/08/2012 08:34 PM, Laurent Pinchart wrote:
> > > The fb_videomode structure stores the front porch and back porch in the
> > > right_margin and left_margin fields respectively. right_margin should
> > > thus be computed with hsync_start - hdisplay, and left_margin with
> > > htotal - hsync_end. The same holds for the vertical direction.
> > > 
> > >         Active               Front           Sync            Back
> > >         Region               Porch                           Porch
> > > 
> > > <-------------------><----------------><-------------><---------------->
> > > 
> > >    //////////////////|
> > >   
> > >   ////////////////// |
> > > 
> > > //////////////////  |..................               ..................
> > > 
> > >                                         _______________
> > > 
> > > <------ xres -------><- right_margin -><- hsync_len -><- left_margin -->
> > > 
> > > <---- hdisplay ----->
> > > <------------ hsync_start ------------>
> > > <--------------------- hsync_end -------------------->
> > > <--------------------------------- htotal ----------------------------->
> > > 
> > > Fix the fb_videomode<->  drm_mode_modeinfo conversion functions
> > > accordingly.
> > > 
> > > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> > > ---
> > > 
> > >   drivers/gpu/drm/exynos/exynos_drm_connector.c |   16 ++++++++--------
> > >   1 files changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > While trying to understand how the fb_videomode and drm_mode_modeinfo
> > > fields map to each other, I found what might be a bug in the Exynos DRM
> > > driver. Could you please check and confirm that my understanding is
> > > correct ?
> > 
> > Good catch. You can refer Documentation/fb/framebuffer.txt to know in
> > detail.
> > 
> > Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>
> 
> Thank you. There's a similar issue in exynos_mipi_dsi_set_display_mode() where 
> front and back porch are inverted. I'll submit a patch for that as well.

Damned. This seems to be a common misunderstanding. I never really
thought about it and always assumed that 'front porch' and 'back porch'
refers to the picture and not to the sync signal. I was so sure that
I didn't even care to google it before I saw this patch.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH] fb: exynos: Fix MIPI/DSI front/back porch settings
From: Laurent Pinchart @ 2012-03-08 15:35 UTC (permalink / raw)
  To: linux-fbdev

The exynos_mipi_dsi_set_main_disp_[hv]porch() functions take front and
back porch arguments in that order. This maps to the fb_videomode
right/lower_margin and left/upper_margin respectively. Fix the caller
accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/exynos/exynos_mipi_dsi_common.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/exynos/exynos_mipi_dsi_common.c b/drivers/video/exynos/exynos_mipi_dsi_common.c
index 14909c1..99f6451 100644
--- a/drivers/video/exynos/exynos_mipi_dsi_common.c
+++ b/drivers/video/exynos/exynos_mipi_dsi_common.c
@@ -738,11 +738,11 @@ int exynos_mipi_dsi_set_display_mode(struct mipi_dsim_device *dsim,
 		if (dsim_config->auto_vertical_cnt = 0) {
 			exynos_mipi_dsi_set_main_disp_vporch(dsim,
 				dsim_config->cmd_allow,
-				timing->upper_margin,
-				timing->lower_margin);
+				timing->lower_margin,
+				timing->upper_margin);
 			exynos_mipi_dsi_set_main_disp_hporch(dsim,
-				timing->left_margin,
-				timing->right_margin);
+				timing->right_margin,
+				timing->left_margin);
 			exynos_mipi_dsi_set_main_disp_sync_area(dsim,
 				timing->vsync_len,
 				timing->hsync_len);
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related

* Re: [PATCH] drm: exynos: Fix fb_videomode <-> drm_mode_modeinfo conversion
From: Laurent Pinchart @ 2012-03-08 15:31 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Inki Dae, linux-fbdev, Seung-Woo Kim, Kyungmin Park, dri-devel
In-Reply-To: <4F58AEAE.4080601@samsung.com>

Hi Joonyoung,

On Thursday 08 March 2012 22:05:50 Joonyoung Shim wrote:
> On 03/08/2012 08:34 PM, Laurent Pinchart wrote:
> > The fb_videomode structure stores the front porch and back porch in the
> > right_margin and left_margin fields respectively. right_margin should
> > thus be computed with hsync_start - hdisplay, and left_margin with
> > htotal - hsync_end. The same holds for the vertical direction.
> > 
> >         Active               Front           Sync            Back
> >         Region               Porch                           Porch
> > 
> > <-------------------><----------------><-------------><---------------->
> > 
> >    //////////////////|
> >   
> >   ////////////////// |
> > 
> > //////////////////  |..................               ..................
> > 
> >                                         _______________
> > 
> > <------ xres -------><- right_margin -><- hsync_len -><- left_margin -->
> > 
> > <---- hdisplay ----->
> > <------------ hsync_start ------------>
> > <--------------------- hsync_end -------------------->
> > <--------------------------------- htotal ----------------------------->
> > 
> > Fix the fb_videomode<->  drm_mode_modeinfo conversion functions
> > accordingly.
> > 
> > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >   drivers/gpu/drm/exynos/exynos_drm_connector.c |   16 ++++++++--------
> >   1 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > While trying to understand how the fb_videomode and drm_mode_modeinfo
> > fields map to each other, I found what might be a bug in the Exynos DRM
> > driver. Could you please check and confirm that my understanding is
> > correct ?
> 
> Good catch. You can refer Documentation/fb/framebuffer.txt to know in
> detail.
> 
> Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>

Thank you. There's a similar issue in exynos_mipi_dsi_set_display_mode() where 
front and back porch are inverted. I'll submit a patch for that as well.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 7/7] OMAPDSS: HDMI: hot plug detect fix
From: Greg KH @ 2012-03-08 15:29 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: stable, linux-fbdev, linux-omap, Rob Clark
In-Reply-To: <1331192113.2354.2.camel@deskari>

On Thu, Mar 08, 2012 at 09:35:13AM +0200, Tomi Valkeinen wrote:
> On Wed, 2012-03-07 at 12:01 -0800, Greg KH wrote:
> > On Thu, Mar 01, 2012 at 02:26:35PM +0200, Tomi Valkeinen wrote:
> > > From: Rob Clark <rob@ti.com>
> > > 
> > > The "OMAPDSS: HDMI: PHY burnout fix" commit switched the HDMI driver
> > > over to using a GPIO for plug detect.  Unfortunately the ->detect()
> > > method was not also updated, causing HDMI to no longer work for the
> > > omapdrm driver (because it would actually check if a connection was
> > > detected before attempting to enable display).
> > > 
> > > Signed-off-by: Rob Clark <rob@ti.com>
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > 
> > You forgot to tell me what the git commit id is for this patch (it's
> > ca888a7958b3d808e4efd08ceff88913f4212c69, right?)
> 
> Yes, that's the one. It wasn't in Linus's tree yet, only in fbdev tree,
> so I wasn't sure what the commit id is.

Then you should not have sent it to me, as if I were to take it then, I
could not have :(

> > And why isn't this needed for the 3.0 kernel as well?
> 
> The detect() function is not present in 3.0, so there was nothing to
> break.

Ok, so everything I've queued up is all that is needed, right?

greg k-h

^ permalink raw reply

* Re: [PATCH] drm: exynos: Fix fb_videomode <-> drm_mode_modeinfo conversion
From: Joonyoung Shim @ 2012-03-08 13:05 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Inki Dae, linux-fbdev, Seung-Woo Kim, Kyungmin Park, dri-devel
In-Reply-To: <1331206495-3853-1-git-send-email-laurent.pinchart@ideasonboard.com>

On 03/08/2012 08:34 PM, Laurent Pinchart wrote:
> The fb_videomode structure stores the front porch and back porch in the
> right_margin and left_margin fields respectively. right_margin should
> thus be computed with hsync_start - hdisplay, and left_margin with
> htotal - hsync_end. The same holds for the vertical direction.
>
>         Active               Front           Sync            Back
>         Region               Porch                           Porch
> <-------------------><----------------><-------------><---------------->
>
>    //////////////////|
>   ////////////////// |
> //////////////////  |..................               ..................
>                                         _______________
>
> <------ xres -------><- right_margin -><- hsync_len -><- left_margin -->
>
> <---- hdisplay ----->
> <------------ hsync_start ------------>
> <--------------------- hsync_end -------------------->
> <--------------------------------- htotal ----------------------------->
>
> Fix the fb_videomode<->  drm_mode_modeinfo conversion functions
> accordingly.
>
> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> ---
>   drivers/gpu/drm/exynos/exynos_drm_connector.c |   16 ++++++++--------
>   1 files changed, 8 insertions(+), 8 deletions(-)
>
> While trying to understand how the fb_videomode and drm_mode_modeinfo fields
> map to each other, I found what might be a bug in the Exynos DRM driver. Could
> you please check and confirm that my understanding is correct ?

Good catch. You can refer Documentation/fb/framebuffer.txt to know in 
detail.

Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>

Thanks.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
> index d620b07..7bb1dca 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
> @@ -52,14 +52,14 @@ convert_to_display_mode(struct drm_display_mode *mode,
>   	mode->vrefresh = timing->refresh;
>
>   	mode->hdisplay = timing->xres;
> -	mode->hsync_start = mode->hdisplay + timing->left_margin;
> +	mode->hsync_start = mode->hdisplay + timing->right_margin;
>   	mode->hsync_end = mode->hsync_start + timing->hsync_len;
> -	mode->htotal = mode->hsync_end + timing->right_margin;
> +	mode->htotal = mode->hsync_end + timing->left_margin;
>
>   	mode->vdisplay = timing->yres;
> -	mode->vsync_start = mode->vdisplay + timing->upper_margin;
> +	mode->vsync_start = mode->vdisplay + timing->lower_margin;
>   	mode->vsync_end = mode->vsync_start + timing->vsync_len;
> -	mode->vtotal = mode->vsync_end + timing->lower_margin;
> +	mode->vtotal = mode->vsync_end + timing->upper_margin;
>
>   	if (timing->vmode&  FB_VMODE_INTERLACED)
>   		mode->flags |= DRM_MODE_FLAG_INTERLACE;
> @@ -81,14 +81,14 @@ convert_to_video_timing(struct fb_videomode *timing,
>   	timing->refresh = drm_mode_vrefresh(mode);
>
>   	timing->xres = mode->hdisplay;
> -	timing->left_margin = mode->hsync_start - mode->hdisplay;
> +	timing->right_margin = mode->hsync_start - mode->hdisplay;
>   	timing->hsync_len = mode->hsync_end - mode->hsync_start;
> -	timing->right_margin = mode->htotal - mode->hsync_end;
> +	timing->left_margin = mode->htotal - mode->hsync_end;
>
>   	timing->yres = mode->vdisplay;
> -	timing->upper_margin = mode->vsync_start - mode->vdisplay;
> +	timing->lower_margin = mode->vsync_start - mode->vdisplay;
>   	timing->vsync_len = mode->vsync_end - mode->vsync_start;
> -	timing->lower_margin = mode->vtotal - mode->vsync_end;
> +	timing->upper_margin = mode->vtotal - mode->vsync_end;
>
>   	if (mode->flags&  DRM_MODE_FLAG_INTERLACE)
>   		timing->vmode = FB_VMODE_INTERLACED;


^ permalink raw reply

* [PATCH] drm: exynos: Fix fb_videomode <-> drm_mode_modeinfo conversion
From: Laurent Pinchart @ 2012-03-08 11:34 UTC (permalink / raw)
  To: dri-devel; +Cc: Inki Dae, Kyungmin Park, linux-fbdev, Seung-Woo Kim

The fb_videomode structure stores the front porch and back porch in the
right_margin and left_margin fields respectively. right_margin should
thus be computed with hsync_start - hdisplay, and left_margin with
htotal - hsync_end. The same holds for the vertical direction.

       Active               Front           Sync            Back
       Region               Porch                           Porch
<-------------------><----------------><-------------><---------------->

  //////////////////|
 ////////////////// |
//////////////////  |..................               ..................
                                       _______________

<------ xres -------><- right_margin -><- hsync_len -><- left_margin -->

<---- hdisplay ----->
<------------ hsync_start ------------>
<--------------------- hsync_end -------------------->
<--------------------------------- htotal ----------------------------->

Fix the fb_videomode <-> drm_mode_modeinfo conversion functions
accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/exynos/exynos_drm_connector.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

While trying to understand how the fb_videomode and drm_mode_modeinfo fields
map to each other, I found what might be a bug in the Exynos DRM driver. Could
you please check and confirm that my understanding is correct ?

diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index d620b07..7bb1dca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -52,14 +52,14 @@ convert_to_display_mode(struct drm_display_mode *mode,
 	mode->vrefresh = timing->refresh;
 
 	mode->hdisplay = timing->xres;
-	mode->hsync_start = mode->hdisplay + timing->left_margin;
+	mode->hsync_start = mode->hdisplay + timing->right_margin;
 	mode->hsync_end = mode->hsync_start + timing->hsync_len;
-	mode->htotal = mode->hsync_end + timing->right_margin;
+	mode->htotal = mode->hsync_end + timing->left_margin;
 
 	mode->vdisplay = timing->yres;
-	mode->vsync_start = mode->vdisplay + timing->upper_margin;
+	mode->vsync_start = mode->vdisplay + timing->lower_margin;
 	mode->vsync_end = mode->vsync_start + timing->vsync_len;
-	mode->vtotal = mode->vsync_end + timing->lower_margin;
+	mode->vtotal = mode->vsync_end + timing->upper_margin;
 
 	if (timing->vmode & FB_VMODE_INTERLACED)
 		mode->flags |= DRM_MODE_FLAG_INTERLACE;
@@ -81,14 +81,14 @@ convert_to_video_timing(struct fb_videomode *timing,
 	timing->refresh = drm_mode_vrefresh(mode);
 
 	timing->xres = mode->hdisplay;
-	timing->left_margin = mode->hsync_start - mode->hdisplay;
+	timing->right_margin = mode->hsync_start - mode->hdisplay;
 	timing->hsync_len = mode->hsync_end - mode->hsync_start;
-	timing->right_margin = mode->htotal - mode->hsync_end;
+	timing->left_margin = mode->htotal - mode->hsync_end;
 
 	timing->yres = mode->vdisplay;
-	timing->upper_margin = mode->vsync_start - mode->vdisplay;
+	timing->lower_margin = mode->vsync_start - mode->vdisplay;
 	timing->vsync_len = mode->vsync_end - mode->vsync_start;
-	timing->lower_margin = mode->vtotal - mode->vsync_end;
+	timing->upper_margin = mode->vtotal - mode->vsync_end;
 
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		timing->vmode = FB_VMODE_INTERLACED;
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related

* [PATCH] add bochs dispi interface framebuffer driver
From: Gerd Hoffmann @ 2012-03-08 10:13 UTC (permalink / raw)
  To: qemu-devel, linux-fbdev; +Cc: Gerd Hoffmann

This patchs adds a frame buffer driver for (virtual/emulated) vga cards
implementing the bochs dispi interface.  Supported hardware are the
bochs vga card with vbe extension and the qemu standard vga.

The driver uses a fixed depth of 32bpp.  Otherwise it supports the full
(but small) feature set of the bochs dispi interface:  Resolution
switching and display panning.  It is tweaked to maximize fbcon speed,
so you'll get the comfort of the framebuffer console in kvm guests
without performance penalty.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/video/Kconfig   |   18 +++
 drivers/video/Makefile  |    1 +
 drivers/video/bochsfb.c |  385 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 404 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/bochsfb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6ca0c40..4d21f90 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -286,6 +286,24 @@ config FB_CIRRUS
 	  Say N unless you have such a graphics board or plan to get one
 	  before you next recompile the kernel.
 
+config FB_BOCHS
+	tristate "Bochs dispi interface support"
+	depends on FB && PCI
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	---help---
+	  This is the frame buffer driver for (virtual/emulated) vga
+          cards implementing the bochs dispi interface.  Supported
+          hardware are the bochs vga card with vbe extension and the
+          qemu standard vga.
+
+          The driver handles the PCI variants only.  It uses a fixed
+          depth of 32bpp, anything else doesn't make sense these days.
+
+          Say Y here if you plan to run the kernel in a virtual machine
+          emulated by bochs or qemu.
+
 config FB_PM2
 	tristate "Permedia2 support"
 	depends on FB && ((AMIGA && BROKEN) || PCI)
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 1426068..a065ad3 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_FB_ARMCLCD)	  += amba-clcd.o
 obj-$(CONFIG_FB_68328)            += 68328fb.o
 obj-$(CONFIG_FB_GBE)              += gbefb.o
 obj-$(CONFIG_FB_CIRRUS)		  += cirrusfb.o
+obj-$(CONFIG_FB_BOCHS)		  += bochsfb.o
 obj-$(CONFIG_FB_ASILIANT)	  += asiliantfb.o
 obj-$(CONFIG_FB_PXA)		  += pxafb.o
 obj-$(CONFIG_FB_PXA168)		  += pxa168fb.o
diff --git a/drivers/video/bochsfb.c b/drivers/video/bochsfb.c
new file mode 100644
index 0000000..18a94dc
--- /dev/null
+++ b/drivers/video/bochsfb.c
@@ -0,0 +1,385 @@
+/*
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License. See the file COPYING in the main directory of this archive for
+ *  more details.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/fb.h>
+#include <linux/pm.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <linux/console.h>
+#include <asm/io.h>
+
+#define VBE_DISPI_IOPORT_INDEX           0x01CE
+#define VBE_DISPI_IOPORT_DATA            0x01CF
+
+#define VBE_DISPI_INDEX_ID               0x0
+#define VBE_DISPI_INDEX_XRES             0x1
+#define VBE_DISPI_INDEX_YRES             0x2
+#define VBE_DISPI_INDEX_BPP              0x3
+#define VBE_DISPI_INDEX_ENABLE           0x4
+#define VBE_DISPI_INDEX_BANK             0x5
+#define VBE_DISPI_INDEX_VIRT_WIDTH       0x6
+#define VBE_DISPI_INDEX_VIRT_HEIGHT      0x7
+#define VBE_DISPI_INDEX_X_OFFSET         0x8
+#define VBE_DISPI_INDEX_Y_OFFSET         0x9
+#define VBE_DISPI_INDEX_VIDEO_MEMORY_64K 0xa
+
+#define VBE_DISPI_ID0                    0xB0C0
+#define VBE_DISPI_ID1                    0xB0C1
+#define VBE_DISPI_ID2                    0xB0C2
+#define VBE_DISPI_ID3                    0xB0C3
+#define VBE_DISPI_ID4                    0xB0C4
+#define VBE_DISPI_ID5                    0xB0C5
+
+#define VBE_DISPI_DISABLED               0x00
+#define VBE_DISPI_ENABLED                0x01
+#define VBE_DISPI_GETCAPS                0x02
+#define VBE_DISPI_8BIT_DAC               0x20
+#define VBE_DISPI_LFB_ENABLED            0x40
+#define VBE_DISPI_NOCLEARMEM             0x80
+
+enum bochs_types {
+	BOCHS_QEMU_STDVGA,
+	BOCHS_UNKNOWN,
+};
+
+static const char *bochs_names[] = {
+	[ BOCHS_QEMU_STDVGA ] = "QEMU standard vga",
+	[ BOCHS_UNKNOWN ]     = "unknown",
+};
+
+static struct fb_fix_screeninfo bochsfb_fix __devinitdata = {
+	.id          = "bochsfb",
+	.type        = FB_TYPE_PACKED_PIXELS,
+	.visual      = FB_VISUAL_TRUECOLOR,
+	.accel       = FB_ACCEL_NONE,
+	.xpanstep    = 1,
+	.ypanstep    = 1,
+};
+
+static struct fb_var_screeninfo bochsfb_var __devinitdata = {
+	.xres           = 1024,
+	.yres           = 768,
+	.bits_per_pixel = 32,
+#ifdef __BIG_ENDIAN
+	.transp         = { .length = 8, .offset =  0 },
+	.red            = { .length = 8, .offset =  8 },
+	.green          = { .length = 8, .offset = 16 },
+	.blue           = { .length = 8, .offset = 24 },
+#else
+	.transp         = { .length = 8, .offset = 24 },
+	.red            = { .length = 8, .offset = 16 },
+	.green          = { .length = 8, .offset =  8 },
+	.blue           = { .length = 8, .offset =  0 },
+#endif
+	.height         = -1,
+	.width          = -1,
+	.vmode          = FB_VMODE_NONINTERLACED,
+	.pixclock       = 10000,
+	.left_margin    = 16,
+	.right_margin   = 16,
+	.upper_margin   = 16,
+	.lower_margin   = 16,
+	.hsync_len      = 8,
+	.vsync_len      = 8,
+};
+
+static char *mode __devinitdata;
+module_param(mode, charp, 0);
+MODULE_PARM_DESC(mode, "Initial video mode e.g. '648x480'");
+
+static u16 bochs_read(u16 reg)
+{
+	outw(reg, VBE_DISPI_IOPORT_INDEX);
+	return inw(VBE_DISPI_IOPORT_DATA);
+}
+
+static void bochs_write(u16 reg, u16 val)
+{
+	outw(reg, VBE_DISPI_IOPORT_INDEX);
+	outw(val, VBE_DISPI_IOPORT_DATA);
+}
+
+static int bochsfb_check_var(struct fb_var_screeninfo *var,
+			     struct fb_info *info)
+{
+	uint32_t x,y, xv,yv, pixels;
+
+	if (var->bits_per_pixel != 32 ||
+	    var->xres > 65535 ||
+	    var->xres_virtual > 65535 ||
+	    (var->vmode & FB_VMODE_MASK) != FB_VMODE_NONINTERLACED)
+		return -EINVAL;
+
+	x  = var->xres & ~0x0f;
+	y  = var->yres & ~0x03;
+	xv = var->xres_virtual & ~0x0f;
+	yv = var->yres_virtual & ~0x03;
+	if (xv < x)
+		xv = x;
+	pixels = info->fix.smem_len * 8 / info->var.bits_per_pixel;
+	yv = pixels / xv;
+	if (y > yv)
+		return -EINVAL;
+
+	var->xres = x;
+	var->yres = y;
+	var->xres_virtual = xv;
+	var->yres_virtual = yv;
+	var->xoffset = 0;
+	var->yoffset = 0;
+
+	return 0;
+}
+
+static int bochsfb_set_par(struct fb_info *info)
+{
+	dev_dbg(info->dev, "set mode: real: %dx%d, virtual: %dx%d\n",
+		info->var.xres, info->var.yres,
+		info->var.xres_virtual, info->var.yres_virtual);
+
+	info->fix.line_length = info->var.xres * info->var.bits_per_pixel / 8;
+
+	bochs_write(VBE_DISPI_INDEX_BPP,         info->var.bits_per_pixel);
+	bochs_write(VBE_DISPI_INDEX_XRES,        info->var.xres);
+	bochs_write(VBE_DISPI_INDEX_YRES,        info->var.yres);
+	bochs_write(VBE_DISPI_INDEX_BANK,        0);
+	bochs_write(VBE_DISPI_INDEX_VIRT_WIDTH,  info->var.xres_virtual);
+	bochs_write(VBE_DISPI_INDEX_VIRT_HEIGHT, info->var.yres_virtual);
+	bochs_write(VBE_DISPI_INDEX_X_OFFSET,    info->var.xoffset);
+	bochs_write(VBE_DISPI_INDEX_Y_OFFSET,    info->var.yoffset);
+
+	bochs_write(VBE_DISPI_INDEX_ENABLE,
+		    VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED);
+	return 0;
+}
+
+static int bochsfb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			     unsigned blue, unsigned transp,
+			     struct fb_info *info)
+{
+	if (regno < 16 && info->var.bits_per_pixel = 32) {
+		red   >>= 8;
+		green >>= 8;
+		blue  >>= 8;
+		((u32 *)(info->pseudo_palette))[regno] +			(red   << info->var.red.offset)   |
+			(green << info->var.green.offset) |
+			(blue  << info->var.blue.offset);
+	}
+	return 0;
+}
+
+static int bochsfb_pan_display(struct fb_var_screeninfo *var,
+			       struct fb_info *info)
+{
+	bochs_write(VBE_DISPI_INDEX_X_OFFSET, var->xoffset);
+	bochs_write(VBE_DISPI_INDEX_Y_OFFSET, var->yoffset);
+	return 0;
+}
+
+static struct fb_ops bochsfb_ops = {
+	.owner	        = THIS_MODULE,
+	.fb_check_var   = bochsfb_check_var,
+	.fb_set_par     = bochsfb_set_par,
+	.fb_setcolreg   = bochsfb_setcolreg,
+	.fb_pan_display = bochsfb_pan_display,
+	.fb_fillrect    = cfb_fillrect,
+	.fb_copyarea    = cfb_copyarea,
+	.fb_imageblit   = cfb_imageblit,
+};
+
+static int __devinit
+bochsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent)
+{
+	struct fb_info *p;
+	unsigned long addr, size, mem;
+	u16 id;
+	int rc = -ENODEV;
+
+	id = bochs_read(VBE_DISPI_INDEX_ID);;
+	mem = bochs_read(VBE_DISPI_INDEX_VIDEO_MEMORY_64K) * 64 * 1024;
+	dev_info(&dp->dev,"Found bochs VGA, ID 0x%x, mem %ldk, type \"%s\".\n",
+		 id, mem / 1024, bochs_names[ent->driver_data]);
+	if ((id & 0xfff0) != VBE_DISPI_ID0) {
+		dev_err(&dp->dev, "ID mismatch\n");
+		goto err_out;
+	}
+
+	if (pci_enable_device(dp) < 0) {
+		dev_err(&dp->dev, "Cannot enable PCI device\n");
+		goto err_out;
+	}
+
+	if ((dp->resource[0].flags & IORESOURCE_MEM) = 0)
+		goto err_disable;
+	addr = pci_resource_start(dp, 0);
+	size = pci_resource_len(dp, 0);
+	if (addr = 0)
+		goto err_disable;
+	if (size != mem) {
+		dev_err(&dp->dev, "Size mismatch: pci=%ld, bochs=%ld\n", size, mem);
+		size = min(size, mem);
+	}
+
+	p = framebuffer_alloc(0, &dp->dev);
+	if (p = NULL) {
+		dev_err(&dp->dev, "Cannot allocate framebuffer structure\n");
+		rc = -ENOMEM;
+		goto err_disable;
+	}
+
+	if (pci_request_region(dp, 0, "bochsfb") != 0) {
+		dev_err(&dp->dev, "Cannot request framebuffer\n");
+		rc = -EBUSY;
+		goto err_release_fb;
+	}
+
+	if (!request_region(VBE_DISPI_IOPORT_INDEX, 2, "bochsfb")) {
+		dev_err(&dp->dev, "Cannot request ioports\n");
+		rc = -EBUSY;
+		goto err_release_pci;
+	}
+
+	p->screen_base = ioremap(addr, size);
+	if (p->screen_base = NULL) {
+		dev_err(&dp->dev, "Cannot map framebuffer\n");
+		rc = -ENOMEM;
+		goto err_release_ports;
+	}
+	memset(p->screen_base, 0, size);
+
+	pci_set_drvdata(dp, p);
+	p->fbops = &bochsfb_ops;
+	p->flags = FBINFO_FLAG_DEFAULT
+		| FBINFO_READS_FAST
+		| FBINFO_HWACCEL_XPAN
+		| FBINFO_HWACCEL_YPAN;
+	p->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
+	p->fix = bochsfb_fix;
+	p->fix.smem_start = addr;
+	p->fix.smem_len = size;
+
+	p->var = bochsfb_var;
+	bochsfb_check_var(&p->var, p);
+	if (mode) {
+		fb_find_mode(&p->var, p, mode, NULL, 0, NULL, 32);
+	}
+
+	if (register_framebuffer(p) < 0) {
+		dev_err(&dp->dev,"Framebuffer failed to register\n");
+		goto err_unmap;
+	}
+
+	dev_info(&dp->dev,"fb%d: bochs VGA frame buffer initialized.\n",
+		 p->node);
+
+	return 0;
+
+ err_unmap:
+	iounmap(p->screen_base);
+ err_release_ports:
+	release_region(VBE_DISPI_IOPORT_INDEX, 2);
+ err_release_pci:
+	pci_release_region(dp, 0);
+ err_release_fb:
+	framebuffer_release(p);
+ err_disable:
+ err_out:
+	return rc;
+}
+
+static void __devexit bochsfb_remove(struct pci_dev *dp)
+{
+	struct fb_info *p = pci_get_drvdata(dp);
+
+	if (p->screen_base = NULL)
+		return;
+	unregister_framebuffer(p);
+	iounmap(p->screen_base);
+	p->screen_base = NULL;
+	release_region(VBE_DISPI_IOPORT_INDEX, 2);
+	pci_release_region(dp, 0);
+	kfree(p->pseudo_palette);
+	framebuffer_release(p);
+}
+
+static struct pci_device_id bochsfb_pci_tbl[] = {
+	{
+		.vendor      = 0x1234,
+		.device      = 0x1111,
+		.subvendor   = 0x1af4,
+		.subdevice   = 0x1100,
+		.driver_data = BOCHS_QEMU_STDVGA,
+	},
+	{
+		.vendor      = 0x1234,
+		.device      = 0x1111,
+		.subvendor   = PCI_ANY_ID,
+		.subdevice   = PCI_ANY_ID,
+		.driver_data = BOCHS_UNKNOWN,
+	},
+	{ /* end of list */ }
+};
+
+MODULE_DEVICE_TABLE(pci, bochsfb_pci_tbl);
+
+static struct pci_driver bochsfb_driver = {
+	.name =		"bochsfb",
+	.id_table =	bochsfb_pci_tbl,
+	.probe =	bochsfb_pci_init,
+	.remove =	__devexit_p(bochsfb_remove),
+};
+
+#ifndef MODULE
+static int __init bochsfb_setup(char *options)
+{
+	char *this_opt;
+
+	if (!options || !*options)
+		return 0;
+
+	while ((this_opt = strsep(&options, ",")) != NULL) {
+		if (!*this_opt)
+			continue;
+		if (!strncmp(this_opt, "mode:", 5))
+			mode = this_opt + 5;
+		else
+			mode = this_opt;
+	}
+	return 0;
+}
+#endif
+
+int __init bochs_init(void)
+{
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("bochsfb", &option))
+		return -ENODEV;
+	bochsfb_setup(option);
+#endif
+	return pci_register_driver(&bochsfb_driver);
+}
+
+module_init(bochs_init);
+
+static void __exit bochsfb_exit(void)
+{
+	pci_unregister_driver(&bochsfb_driver);
+}
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gerd Hoffmann <kraxel@redhat.com>");
+MODULE_DESCRIPTION("bochs dispi interface framebuffer driver");
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 16/21] OMAPDSS: handle output-driver reg/unreg more dynamically
From: Archit Taneja @ 2012-03-08  9:51 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1331199297.2354.62.camel@deskari>

On Thursday 08 March 2012 03:04 PM, Tomi Valkeinen wrote:
> On Thu, 2012-03-08 at 14:52 +0530, Archit Taneja wrote:
>> On Thursday 08 March 2012 02:16 PM, Tomi Valkeinen wrote:
>>> On Thu, 2012-03-08 at 14:04 +0530, Archit Taneja wrote:

<snip>

>> Oh okay. But the comment after the patch set still says "It's ok if the
>> output-driver register fails.", we could change it to "It's ok if the
>> output-driver probe fails."
>
> Well, I guess this goes into nitpicking area, but if there are no
> devices, probe is not called at all. So I think it's the driver register
> that fails in that case. If there is a device, and it is probed, and
> that fails, then it's probe which fails.

Yes, that's fair enough I guess.

<snip>

>>
>> If we ensure that none of our probes return ENODEV(even though it may
>> make sense to return it if a func within probe fails), we could
>> differentiate between the 2 cases, right?
>
> True, I thought about that. But we can never be sure that the functions
> called by the probe (clk_get, or whatever) won't return ENODEV. Of
> course, we could check what they return, and change the error to
> something else, but I'm not sure if that's good either.

That's true.

<snip>

> Alternatively, if the platform driver code was changed to tell us
> clearly if it was the probe that failed or if there just weren't any
> devices, we could also use that.

Yes, I wonder how that could be done.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH 16/21] OMAPDSS: handle output-driver reg/unreg more dynamically
From: Tomi Valkeinen @ 2012-03-08  9:34 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F587A62.3020202@ti.com>

[-- Attachment #1: Type: text/plain, Size: 5275 bytes --]

On Thu, 2012-03-08 at 14:52 +0530, Archit Taneja wrote:
> On Thursday 08 March 2012 02:16 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-03-08 at 14:04 +0530, Archit Taneja wrote:
> >> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
> >
> >>> -	r = hdmi_init_platform_driver();
> >>> -	if (r) {
> >>> -		DSSERR("Failed to initialize hdmi\n");
> >>> -		goto err_hdmi;
> >>> +	/*
> >>> +	 * It's ok if the output-driver register fails. It happens, for example,
> >>> +	 * when there is no output-device (e.g. SDI for OMAP4).
> >>> +	 */
> >>
> >> Suppose we do a omap2plus_defconfig, CONFIG_OMAP2_DSS_SDI would be
> >> selected, and sdi.c would be built, if we boot on OMAP4, why would a sdi
> >> driver register cause a failure? Wouldn't the sdi driver just get
> >> registered, and wait till eternity for the corresponding sdi platform
> >> device to get registered?
> >
> > No. Well, yes.
> >
> > Currently we use platform_driver_register() to register the drivers, and
> > it does just what you described. But a few patches later I change
> > platform_driver_register() to platform_driver_probe(), which will return
> > ENODEV if there are no matching devices for the driver.
> >
> > I originally had the platform_driver_probe() patch before this patch,
> > and thus the comment above made sense. Now the patch is after this
> > patch, so the comment is not exactly right until the probe patch is also
> > applied.
> 
> Oh okay. But the comment after the patch set still says "It's ok if the 
> output-driver register fails.", we could change it to "It's ok if the 
> output-driver probe fails."

Well, I guess this goes into nitpicking area, but if there are no
devices, probe is not called at all. So I think it's the driver register
that fails in that case. If there is a device, and it is probed, and
that fails, then it's probe which fails.

> > The point with platform_driver_probe() is that it can be used with
> > non-removable devices which are created at boot time, like the DSS
> > components. With platform_driver_probe() the probe function is called
> > only at that one time, and never afterwards. So probe can be in __init
> > section, and thrown away after init.
> 
> So platform_driver_probe() is like a driver_register() + probe().

Yes. Well, when platform_driver_register() is called, and the devices
are already present, it will call the probe also. So in that sense they
are similar. The difference is that for platform_driver_register() the
probe pointer must be in the driver struct, and it stays there even
after init. For platform_driver_probe(), the probe pointer is given as
an argument to the function, and thus it's not stored anywhere and can
be thrown away afterwards.

> Okay, in our case, all the devices are created at boot time, and if 
> omapdss were a module, the probes would have been thrown away after 
> module_init(), right?

Yes. If omapdss is a module, the functions marked with __init are
discarded after the module_init is done. If omapdss is built-in, the
__init funcs are thrown away after the kernel's init done.

> > One side effect of using platform_driver_probe() is that it returns
> > ENODEV is there are no devices. In a simple module, the error can be
> > then returned from module_init, thus causing the whole module to be
> > unloaded. Our case is a bit more complex as we have multiple drivers in
> > the same module.
> >
> > A downside with that is that we don't really know if the ENODEV error
> > happened because there were no devices (which is ok), or if it came from
> > probe function (which is not so ok). However, I thought that it doesn't
> > matter if an output driver has failed. We can still continue with the
> > other output drivers just fine.
> 
> If we ensure that none of our probes return ENODEV(even though it may 
> make sense to return it if a func within probe fails), we could 
> differentiate between the 2 cases, right?

True, I thought about that. But we can never be sure that the functions
called by the probe (clk_get, or whatever) won't return ENODEV. Of
course, we could check what they return, and change the error to
something else, but I'm not sure if that's good either.

> >
> > Actually, there is a small problem. If, for example, DSI driver fails to
> > load, and DPI driver tries to use DSI PLL...
> 
> If we could differentiate between an error occuring because the device 
> doesn't exist and an error occuring because the probe failed, we could 
> bail out if any of the probes fail, right?

Yes, but it feels a bit hackish to try to use the error as I pointed out
above. So I'd rather go the other way: the drivers should somehow
register the stuff they offer, so for example when the DSI1 is probed,
it should register the DSI1 PLL to dss core. And DPI would have to ask
for the DSI1 PLL from dss core.

That, of course, is not a trivial change, so for the moment this stuff
is slightly broken in error cases. Perhaps we could figure out some kind
of clean hack for that...

Alternatively, if the platform driver code was changed to tell us
clearly if it was the probe that failed or if there just weren't any
devices, we could also use that.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 16/21] OMAPDSS: handle output-driver reg/unreg more dynamically
From: Archit Taneja @ 2012-03-08  9:34 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1331196364.2354.49.camel@deskari>

On Thursday 08 March 2012 02:16 PM, Tomi Valkeinen wrote:
> On Thu, 2012-03-08 at 14:04 +0530, Archit Taneja wrote:
>> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
>
>>> -	r = hdmi_init_platform_driver();
>>> -	if (r) {
>>> -		DSSERR("Failed to initialize hdmi\n");
>>> -		goto err_hdmi;
>>> +	/*
>>> +	 * It's ok if the output-driver register fails. It happens, for example,
>>> +	 * when there is no output-device (e.g. SDI for OMAP4).
>>> +	 */
>>
>> Suppose we do a omap2plus_defconfig, CONFIG_OMAP2_DSS_SDI would be
>> selected, and sdi.c would be built, if we boot on OMAP4, why would a sdi
>> driver register cause a failure? Wouldn't the sdi driver just get
>> registered, and wait till eternity for the corresponding sdi platform
>> device to get registered?
>
> No. Well, yes.
>
> Currently we use platform_driver_register() to register the drivers, and
> it does just what you described. But a few patches later I change
> platform_driver_register() to platform_driver_probe(), which will return
> ENODEV if there are no matching devices for the driver.
>
> I originally had the platform_driver_probe() patch before this patch,
> and thus the comment above made sense. Now the patch is after this
> patch, so the comment is not exactly right until the probe patch is also
> applied.

Oh okay. But the comment after the patch set still says "It's ok if the 
output-driver register fails.", we could change it to "It's ok if the 
output-driver probe fails."

>
> The point with platform_driver_probe() is that it can be used with
> non-removable devices which are created at boot time, like the DSS
> components. With platform_driver_probe() the probe function is called
> only at that one time, and never afterwards. So probe can be in __init
> section, and thrown away after init.

So platform_driver_probe() is like a driver_register() + probe().

Okay, in our case, all the devices are created at boot time, and if 
omapdss were a module, the probes would have been thrown away after 
module_init(), right?

>
> One side effect of using platform_driver_probe() is that it returns
> ENODEV is there are no devices. In a simple module, the error can be
> then returned from module_init, thus causing the whole module to be
> unloaded. Our case is a bit more complex as we have multiple drivers in
> the same module.
>
> A downside with that is that we don't really know if the ENODEV error
> happened because there were no devices (which is ok), or if it came from
> probe function (which is not so ok). However, I thought that it doesn't
> matter if an output driver has failed. We can still continue with the
> other output drivers just fine.

If we ensure that none of our probes return ENODEV(even though it may 
make sense to return it if a func within probe fails), we could 
differentiate between the 2 cases, right?

>
> Actually, there is a small problem. If, for example, DSI driver fails to
> load, and DPI driver tries to use DSI PLL...

If we could differentiate between an error occuring because the device 
doesn't exist and an error occuring because the probe failed, we could 
bail out if any of the probes fail, right?

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [RFC PATCH] video:backlight: add dimming sysfs node
From: Lars-Peter Clausen @ 2012-03-08  9:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4F58573C.3020407@samsung.com>

On 03/08/2012 07:52 AM, Donghwa Lee wrote:
> In backlight class, update_status() callback function is mostly used to change
> backlight brightness. When platform enter the dimming state, it is usually used.
> But, I think dimming state can be defined variety of method including brightness.
> So, it is need to differentiated node from brightness node.

What do you mean by dimming? Completely blank the display or just lower the
brightness to a certain level > 0? In the former case the bl_power sysfs
node already exposes such functionality. In the later case can you give an
example how this will be used and how a typical driver would implement this
functionality?

> 
> In set_dimming() callback function, developers can define variety dimming functions.
> 
> Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/video/backlight/backlight.c |   37 +++++++++++++++++++++++++++++++++++
>  include/linux/backlight.h           |    9 ++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index bf5b1ec..44a77e4 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -101,6 +101,43 @@ static void backlight_generate_event(struct backlight_device *bd,
>  	sysfs_notify(&bd->dev.kobj, NULL, "actual_brightness");
>  }
>  
> +static ssize_t backlight_store_dimming(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	int rc;
> +	struct backlight_device *bd = to_backlight_device(dev);
> +	unsigned long dimming;
> +
> +	rc = strict_strtoul(buf, 0, &dimming);
> +	if (rc)
> +		return rc;
> +
> +	if (dimming < 0)
> +		rc = -EINVAL;
> +	else {
> +		pr_debug("set dimming mode\n");
> +
> +		if (dimming)
> +			bd->props.dimming = true;
> +		else
> +			bd->props.dimming = false;
> +
> +		backlight_set_dimming(bd);
> +
> +		rc = count;
> +	}
> +
> +	return rc;
> +}
> +
> +static ssize_t backlight_show_dimming(struct device *dev,
> +		struct device_attribute *attr,char *buf)
> +{
> +	struct backlight_device *bd = to_backlight_device(dev);
> +
> +	return sprintf(buf, "%d\n", bd->props.dimming);
> +}
> +
>  static ssize_t backlight_show_power(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 5ffc6dd..823717e 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -55,10 +55,13 @@ struct backlight_ops {
>  	/* Check if given framebuffer device is the one bound to this backlight;
>  	   return 0 if not, !=0 if it is. If NULL, backlight always matches the fb. */
>  	int (*check_fb)(struct backlight_device *, struct fb_info *);
> +	/* Notify the backlight driver to enter the dimming state */
> +	int (*set_dimming)(struct backlight_device *);
>  };
>  
>  /* This structure defines all the properties of a backlight */
>  struct backlight_properties {
> +	bool dimming;
>  	/* Current User requested brightness (0 - max_brightness) */
>  	int brightness;
>  	/* Maximal value for brightness (read-only) */
> @@ -111,6 +114,12 @@ static inline void backlight_update_status(struct backlight_device *bd)
>  	mutex_unlock(&bd->update_lock);
>  }
>  
> +static inline void backlight_set_dimming(struct backlight_device *bd)
> +{
> +	if (bd->ops && bd->ops->set_dimming)
> +		bd->ops->set_dimming(bd);
> +}
> +
>  extern struct backlight_device *backlight_device_register(const char *name,
>  	struct device *dev, void *devdata, const struct backlight_ops *ops,
>  	const struct backlight_properties *props);


^ permalink raw reply

* Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess
From: Archit Taneja @ 2012-03-08  8:55 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1331195632.2354.37.camel@deskari>

On Thursday 08 March 2012 02:03 PM, Tomi Valkeinen wrote:
> On Thu, 2012-03-08 at 13:47 +0530, Archit Taneja wrote:
>> On Thursday 08 March 2012 01:32 PM, Tomi Valkeinen wrote:
>
>>>> why do we check board_data being NULL here and not in omap_display_init()?
>>>
>>> I added it for DT case, because then we don't have board_data for the
>>> devices defined in the DT data. However, for now we always have the
>>> board_data, and in this patch I should just move the code. So I'll
>>> remove the check, and add it later with DT code if needed.
>>
>> Ok. When DT will be in use, would omap_display_init() be called or not?
>
> No. Currently the board files create and fill the board_data, and then
> call omap_display_init.
>
> With DT, the DT data will contain all the dynamic, per-board
> information. Something like:
>
> dss {
> 	dpi {
> 		dvi {
> 			pd-gpio =<10>;
> 			...
> 		};
> 	};
>
> 	dsi@1 {
> 		taal {
> 			reset-gpio =<20>;
> 			...
> 		};
> 	}
>
> 	...
> };
>
> The DT data will be passed individually to each dss driver (i.e. dsi
> driver will get its DT node, etc.). The drivers will read the data, and
> initialize themselves with that, more or less the same manner they'd do
> with the board_data from board files.
>
> However, we currently have this "omapdss" device, which is not a hwmod
> device at all. In the long run I think the omapdss device should be
> removed, but for now we need it. And device has to be created in the
> arch code, the same way it's now created in omap_display_init().
>
> So with DT we need a new func, omap_display_init_dt() or such, which
> creates the omapdss device, and also creates a board_data which contains
> the ctx_loss etc function pointers. But the board data won't have any
> display data, those come directly from DT data.
>
> It's a bit messy solution, but it should allow us to have both DT and
> non-DT working at the same time, with quite minimal changes to the board
> files.

Okay, thanks for the clarification.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH 16/21] OMAPDSS: handle output-driver reg/unreg more dynamically
From: Archit Taneja @ 2012-03-08  8:46 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1331124290-6285-17-git-send-email-tomi.valkeinen@ti.com>

On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
> Initialize and uninitialize the output drivers by using arrays of
> pointers to the init/uninit functions. This simplifies the code
> slightly.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/core.c |  111 +++++++++++++++++++++-------------------
>   drivers/video/omap2/dss/dss.h  |   41 ---------------
>   2 files changed, 59 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index 654962a..ac4f2cb 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> @@ -503,10 +503,54 @@ static int omap_dss_bus_register(void)
>   }
>
>   /* INIT */
> +static int (*dss_output_drv_reg_funcs[])(void) __initdata = {
> +#ifdef CONFIG_OMAP2_DSS_DPI
> +	dpi_init_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_SDI
> +	sdi_init_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_RFBI
> +	rfbi_init_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_VENC
> +	venc_init_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_DSI
> +	dsi_init_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP4_DSS_HDMI
> +	hdmi_init_platform_driver,
> +#endif
> +};
> +
> +static void (*dss_output_drv_unreg_funcs[])(void) __exitdata = {
> +#ifdef CONFIG_OMAP2_DSS_DPI
> +	dpi_uninit_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_SDI
> +	sdi_uninit_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_RFBI
> +	rfbi_uninit_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_VENC
> +	venc_uninit_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP2_DSS_DSI
> +	dsi_uninit_platform_driver,
> +#endif
> +#ifdef CONFIG_OMAP4_DSS_HDMI
> +	hdmi_uninit_platform_driver,
> +#endif
> +};
> +
> +static bool dss_output_drv_loaded[ARRAY_SIZE(dss_output_drv_reg_funcs)];
>
>   static int __init omap_dss_register_drivers(void)
>   {
>   	int r;
> +	int i;
>
>   	r = platform_driver_probe(&omap_dss_driver, omap_dss_probe);
>   	if (r)
> @@ -524,56 +568,18 @@ static int __init omap_dss_register_drivers(void)
>   		goto err_dispc;
>   	}
>
> -	r = dpi_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize dpi platform driver\n");
> -		goto err_dpi;
> -	}
> -
> -	r = sdi_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize sdi platform driver\n");
> -		goto err_sdi;
> -	}
> -
> -	r = rfbi_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize rfbi platform driver\n");
> -		goto err_rfbi;
> -	}
> -
> -	r = venc_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize venc platform driver\n");
> -		goto err_venc;
> -	}
> -
> -	r = dsi_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize DSI platform driver\n");
> -		goto err_dsi;
> -	}
> -
> -	r = hdmi_init_platform_driver();
> -	if (r) {
> -		DSSERR("Failed to initialize hdmi\n");
> -		goto err_hdmi;
> +	/*
> +	 * It's ok if the output-driver register fails. It happens, for example,
> +	 * when there is no output-device (e.g. SDI for OMAP4).
> +	 */

Suppose we do a omap2plus_defconfig, CONFIG_OMAP2_DSS_SDI would be 
selected, and sdi.c would be built, if we boot on OMAP4, why would a sdi 
driver register cause a failure? Wouldn't the sdi driver just get 
registered, and wait till eternity for the corresponding sdi platform 
device to get registered?

Archit

> +	for (i = 0; i<  ARRAY_SIZE(dss_output_drv_reg_funcs); ++i) {
> +		r = dss_output_drv_reg_funcs[i]();
> +		if (r = 0)
> +			dss_output_drv_loaded[i] = true;
>   	}
>
>   	return 0;
>
> -err_hdmi:
> -	dsi_uninit_platform_driver();
> -err_dsi:
> -	venc_uninit_platform_driver();
> -err_venc:
> -	rfbi_uninit_platform_driver();
> -err_rfbi:
> -	sdi_uninit_platform_driver();
> -err_sdi:
> -	dpi_uninit_platform_driver();
> -err_dpi:
> -	dispc_uninit_platform_driver();
>   err_dispc:
>   	dss_uninit_platform_driver();
>   err_dss:
> @@ -584,12 +590,13 @@ err_dss:
>
>   static void __exit omap_dss_unregister_drivers(void)
>   {
> -	hdmi_uninit_platform_driver();
> -	dsi_uninit_platform_driver();
> -	venc_uninit_platform_driver();
> -	rfbi_uninit_platform_driver();
> -	sdi_uninit_platform_driver();
> -	dpi_uninit_platform_driver();
> +	int i;
> +
> +	for (i = 0; i<  ARRAY_SIZE(dss_output_drv_unreg_funcs); ++i) {
> +		if (dss_output_drv_loaded[i])
> +			dss_output_drv_unreg_funcs[i]();
> +	}
> +
>   	dispc_uninit_platform_driver();
>   	dss_uninit_platform_driver();
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 24aadde..af7bed1 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -265,14 +265,9 @@ int dss_calc_clock_div(bool is_tft, unsigned long req_pck,
>   		struct dispc_clock_info *dispc_cinfo);
>
>   /* SDI */
> -#ifdef CONFIG_OMAP2_DSS_SDI
>   int sdi_init_platform_driver(void);
>   void sdi_uninit_platform_driver(void);
>   int sdi_init_display(struct omap_dss_device *display);
> -#else
> -static inline int sdi_init_platform_driver(void) { return 0; }
> -static inline void sdi_uninit_platform_driver(void) { }
> -#endif
>
>   /* DSI */
>   #ifdef CONFIG_OMAP2_DSS_DSI
> @@ -309,13 +304,6 @@ void dsi_wait_pll_hsdiv_dispc_active(struct platform_device *dsidev);
>   void dsi_wait_pll_hsdiv_dsi_active(struct platform_device *dsidev);
>   struct platform_device *dsi_get_dsidev_from_id(int module);
>   #else
> -static inline int dsi_init_platform_driver(void)
> -{
> -	return 0;
> -}
> -static inline void dsi_uninit_platform_driver(void)
> -{
> -}
>   static inline int dsi_runtime_get(struct platform_device *dsidev)
>   {
>   	return 0;
> @@ -372,14 +360,9 @@ static inline struct platform_device *dsi_get_dsidev_from_id(int module)
>   #endif
>
>   /* DPI */
> -#ifdef CONFIG_OMAP2_DSS_DPI
>   int dpi_init_platform_driver(void);
>   void dpi_uninit_platform_driver(void);
>   int dpi_init_display(struct omap_dss_device *dssdev);
> -#else
> -static inline int dpi_init_platform_driver(void) { return 0; }
> -static inline void dpi_uninit_platform_driver(void) { }
> -#endif
>
>   /* DISPC */
>   int dispc_init_platform_driver(void);
> @@ -456,13 +439,6 @@ void venc_dump_regs(struct seq_file *s);
>   int venc_init_display(struct omap_dss_device *display);
>   unsigned long venc_get_pixel_clock(void);
>   #else
> -static inline int venc_init_platform_driver(void)
> -{
> -	return 0;
> -}
> -static inline void venc_uninit_platform_driver(void)
> -{
> -}
>   static inline unsigned long venc_get_pixel_clock(void)
>   {
>   	WARN("%s: VENC not compiled in, returning pclk as 0\n", __func__);
> @@ -482,13 +458,6 @@ static inline int hdmi_init_display(struct omap_dss_device *dssdev)
>   {
>   	return 0;
>   }
> -static inline int hdmi_init_platform_driver(void)
> -{
> -	return 0;
> -}
> -static inline void hdmi_uninit_platform_driver(void)
> -{
> -}
>   static inline unsigned long hdmi_get_pixel_clock(void)
>   {
>   	WARN("%s: HDMI not compiled in, returning pclk as 0\n", __func__);
> @@ -506,20 +475,10 @@ int hdmi_panel_init(void);
>   void hdmi_panel_exit(void);
>
>   /* RFBI */
> -#ifdef CONFIG_OMAP2_DSS_RFBI
>   int rfbi_init_platform_driver(void);
>   void rfbi_uninit_platform_driver(void);
>   void rfbi_dump_regs(struct seq_file *s);
>   int rfbi_init_display(struct omap_dss_device *display);
> -#else
> -static inline int rfbi_init_platform_driver(void)
> -{
> -	return 0;
> -}
> -static inline void rfbi_uninit_platform_driver(void)
> -{
> -}
> -#endif
>
>
>   #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS


^ permalink raw reply

* Re: [PATCH 16/21] OMAPDSS: handle output-driver reg/unreg more dynamically
From: Tomi Valkeinen @ 2012-03-08  8:46 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F586EFD.1020208@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]

On Thu, 2012-03-08 at 14:04 +0530, Archit Taneja wrote:
> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:

> > -	r = hdmi_init_platform_driver();
> > -	if (r) {
> > -		DSSERR("Failed to initialize hdmi\n");
> > -		goto err_hdmi;
> > +	/*
> > +	 * It's ok if the output-driver register fails. It happens, for example,
> > +	 * when there is no output-device (e.g. SDI for OMAP4).
> > +	 */
> 
> Suppose we do a omap2plus_defconfig, CONFIG_OMAP2_DSS_SDI would be 
> selected, and sdi.c would be built, if we boot on OMAP4, why would a sdi 
> driver register cause a failure? Wouldn't the sdi driver just get 
> registered, and wait till eternity for the corresponding sdi platform 
> device to get registered?

No. Well, yes.

Currently we use platform_driver_register() to register the drivers, and
it does just what you described. But a few patches later I change
platform_driver_register() to platform_driver_probe(), which will return
ENODEV if there are no matching devices for the driver.

I originally had the platform_driver_probe() patch before this patch,
and thus the comment above made sense. Now the patch is after this
patch, so the comment is not exactly right until the probe patch is also
applied.

The point with platform_driver_probe() is that it can be used with
non-removable devices which are created at boot time, like the DSS
components. With platform_driver_probe() the probe function is called
only at that one time, and never afterwards. So probe can be in __init
section, and thrown away after init.

One side effect of using platform_driver_probe() is that it returns
ENODEV is there are no devices. In a simple module, the error can be
then returned from module_init, thus causing the whole module to be
unloaded. Our case is a bit more complex as we have multiple drivers in
the same module.

A downside with that is that we don't really know if the ENODEV error
happened because there were no devices (which is ok), or if it came from
probe function (which is not so ok). However, I thought that it doesn't
matter if an output driver has failed. We can still continue with the
other output drivers just fine.

Actually, there is a small problem. If, for example, DSI driver fails to
load, and DPI driver tries to use DSI PLL...

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess
From: Tomi Valkeinen @ 2012-03-08  8:33 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F586B28.1050403@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1870 bytes --]

On Thu, 2012-03-08 at 13:47 +0530, Archit Taneja wrote:
> On Thursday 08 March 2012 01:32 PM, Tomi Valkeinen wrote:

> >> why do we check board_data being NULL here and not in omap_display_init()?
> >
> > I added it for DT case, because then we don't have board_data for the
> > devices defined in the DT data. However, for now we always have the
> > board_data, and in this patch I should just move the code. So I'll
> > remove the check, and add it later with DT code if needed.
> 
> Ok. When DT will be in use, would omap_display_init() be called or not?

No. Currently the board files create and fill the board_data, and then
call omap_display_init.

With DT, the DT data will contain all the dynamic, per-board
information. Something like:

dss {
	dpi {
		dvi {
			pd-gpio = <10>;
			...
		};
	};

	dsi@1 {
		taal {
			reset-gpio = <20>;
			...
		};
	}

	...
};

The DT data will be passed individually to each dss driver (i.e. dsi
driver will get its DT node, etc.). The drivers will read the data, and
initialize themselves with that, more or less the same manner they'd do
with the board_data from board files.

However, we currently have this "omapdss" device, which is not a hwmod
device at all. In the long run I think the omapdss device should be
removed, but for now we need it. And device has to be created in the
arch code, the same way it's now created in omap_display_init().

So with DT we need a new func, omap_display_init_dt() or such, which
creates the omapdss device, and also creates a board_data which contains
the ctx_loss etc function pointers. But the board data won't have any
display data, those come directly from DT data.

It's a bit messy solution, but it should allow us to have both DT and
non-DT working at the same time, with quite minimal changes to the board
files.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess
From: Archit Taneja @ 2012-03-08  8:29 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1331193777.2354.21.camel@deskari>

On Thursday 08 March 2012 01:32 PM, Tomi Valkeinen wrote:
> On Wed, 2012-03-07 at 23:41 +0530, Archit Taneja wrote:
>> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
>>> The omapdss pdata handling is a mess. This is more evident when trying
>>> to use device tree for DSS, as we don't have platform data anymore in
>>> that case. This patch cleans the pdata handling by:
>>>
>>> - Remove struct omap_display_platform_data. It was used just as a
>>>     wrapper for struct omap_dss_board_info.
>>> - Pass the platform data only to omapdss device. The drivers for omap
>>>     dss hwmods do not need the platform data. This should also work better
>>>     for DT, as we can create omapdss device programmatically in generic omap
>>>     boot code, and thus we can pass the pdata to it.
>>> - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads
>>>     that the dss hwmod drivers can call.
>>>
>>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>>> ---
>>>    arch/arm/mach-omap2/display.c   |   37 ++++++++++++++++++-------------------
>>>    drivers/video/omap2/dss/core.c  |   35 +++++++++++++++++++++++++++++++++++
>>>    drivers/video/omap2/dss/dispc.c |   21 ++-------------------
>>>    drivers/video/omap2/dss/dsi.c   |   17 +++--------------
>>>    drivers/video/omap2/dss/dss.h   |    3 +++
>>>    drivers/video/omap2/dss/hdmi.c  |    2 --
>>>    include/video/omapdss.h         |    5 -----
>>>    7 files changed, 61 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>>> index 3677b1f..279c124 100644
>>> --- a/arch/arm/mach-omap2/display.c
>>> +++ b/arch/arm/mach-omap2/display.c
>>> @@ -185,10 +185,23 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
>>>    	struct omap_hwmod *oh;
>>>    	struct platform_device *pdev;
>>>    	int i, oh_count;
>>> -	struct omap_display_platform_data pdata;
>>>    	const struct omap_dss_hwmod_data *curr_dss_hwmod;
>>>
>>> -	memset(&pdata, 0, sizeof(pdata));
>>> +	/* create omapdss device */
>>> +
>>> +	board_data->dsi_enable_pads = omap_dsi_enable_pads;
>>> +	board_data->dsi_disable_pads = omap_dsi_disable_pads;
>>> +	board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count;
>>
>> Why are the checks for board data being NULL removed here?
>
> We didn't check if board_data is NULL in the earlier version either. And
> I don't think there's need to check that, because if the board file
> calls this function, it should also give the board data.
>
> However, the earlier version didn't set the func pointers if the func
> pointer in the board_data was != NULL. Did you mean that? I removed that
> check, as I don't see a need for it. The func pointers should be set by
> this function, and I don't see why the board file would need to use its
> own versions.

Yes, I had meant the function pointers being != NULL, yes it doesn't 
make sense for the board file to populate these.

>
>>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>>> index 8613f86..3efd473 100644
>>> --- a/drivers/video/omap2/dss/core.c
>>> +++ b/drivers/video/omap2/dss/core.c
>>> @@ -87,6 +87,41 @@ struct regulator *dss_get_vdds_sdi(void)
>>>    	return reg;
>>>    }
>>>
>>> +int dss_get_ctx_loss_count(struct device *dev)
>>> +{
>>> +	struct omap_dss_board_info *board_data = core.pdev->dev.platform_data;
>>> +	int cnt;
>>> +
>>> +	if (!board_data || !board_data->get_context_loss_count)
>>> +		return -ENOENT;
>>
>> why do we check board_data being NULL here and not in omap_display_init()?
>
> I added it for DT case, because then we don't have board_data for the
> devices defined in the DT data. However, for now we always have the
> board_data, and in this patch I should just move the code. So I'll
> remove the check, and add it later with DT code if needed.

Ok. When DT will be in use, would omap_display_init() be called or not?

>
> (and actually, I don' think the check is needed in DT case either...)

I don't know how things would work in DT case. So I'm not sure what's 
happening here.

Archit

>
>   Tomi
>
>
>
>


^ permalink raw reply

* Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess
From: Tomi Valkeinen @ 2012-03-08  8:02 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <4F57A4E5.6070201@ti.com>

[-- Attachment #1: Type: text/plain, Size: 3802 bytes --]

On Wed, 2012-03-07 at 23:41 +0530, Archit Taneja wrote:
> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
> > The omapdss pdata handling is a mess. This is more evident when trying
> > to use device tree for DSS, as we don't have platform data anymore in
> > that case. This patch cleans the pdata handling by:
> >
> > - Remove struct omap_display_platform_data. It was used just as a
> >    wrapper for struct omap_dss_board_info.
> > - Pass the platform data only to omapdss device. The drivers for omap
> >    dss hwmods do not need the platform data. This should also work better
> >    for DT, as we can create omapdss device programmatically in generic omap
> >    boot code, and thus we can pass the pdata to it.
> > - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads
> >    that the dss hwmod drivers can call.
> >
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> >   arch/arm/mach-omap2/display.c   |   37 ++++++++++++++++++-------------------
> >   drivers/video/omap2/dss/core.c  |   35 +++++++++++++++++++++++++++++++++++
> >   drivers/video/omap2/dss/dispc.c |   21 ++-------------------
> >   drivers/video/omap2/dss/dsi.c   |   17 +++--------------
> >   drivers/video/omap2/dss/dss.h   |    3 +++
> >   drivers/video/omap2/dss/hdmi.c  |    2 --
> >   include/video/omapdss.h         |    5 -----
> >   7 files changed, 61 insertions(+), 59 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> > index 3677b1f..279c124 100644
> > --- a/arch/arm/mach-omap2/display.c
> > +++ b/arch/arm/mach-omap2/display.c
> > @@ -185,10 +185,23 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> >   	struct omap_hwmod *oh;
> >   	struct platform_device *pdev;
> >   	int i, oh_count;
> > -	struct omap_display_platform_data pdata;
> >   	const struct omap_dss_hwmod_data *curr_dss_hwmod;
> >
> > -	memset(&pdata, 0, sizeof(pdata));
> > +	/* create omapdss device */
> > +
> > +	board_data->dsi_enable_pads = omap_dsi_enable_pads;
> > +	board_data->dsi_disable_pads = omap_dsi_disable_pads;
> > +	board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count;
> 
> Why are the checks for board data being NULL removed here?

We didn't check if board_data is NULL in the earlier version either. And
I don't think there's need to check that, because if the board file
calls this function, it should also give the board data.

However, the earlier version didn't set the func pointers if the func
pointer in the board_data was != NULL. Did you mean that? I removed that
check, as I don't see a need for it. The func pointers should be set by
this function, and I don't see why the board file would need to use its
own versions.

> > diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> > index 8613f86..3efd473 100644
> > --- a/drivers/video/omap2/dss/core.c
> > +++ b/drivers/video/omap2/dss/core.c
> > @@ -87,6 +87,41 @@ struct regulator *dss_get_vdds_sdi(void)
> >   	return reg;
> >   }
> >
> > +int dss_get_ctx_loss_count(struct device *dev)
> > +{
> > +	struct omap_dss_board_info *board_data = core.pdev->dev.platform_data;
> > +	int cnt;
> > +
> > +	if (!board_data || !board_data->get_context_loss_count)
> > +		return -ENOENT;
> 
> why do we check board_data being NULL here and not in omap_display_init()?

I added it for DT case, because then we don't have board_data for the
devices defined in the DT data. However, for now we always have the
board_data, and in this patch I should just move the code. So I'll
remove the check, and add it later with DT code if needed.

(and actually, I don' think the check is needed in DT case either...)

 Tomi





[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 01/21] OMAPDSS: panel-dvi: add PD gpio handling
From: Tomi Valkeinen @ 2012-03-08  7:54 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F579F1B.7040503@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

On Wed, 2012-03-07 at 23:17 +0530, Archit Taneja wrote:
> Hi,
> 
> On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote:
> > The driver for the DVI framer should handle the power-down signal of the
> > framer, instead of the current way of handling it in the board files.
> 
> What does framer mean?

I don't know where the word has come, and I can't find it in the TFP410
documentation. I guess the idea with the word was that the chip
"frames", i.e. packetizes, the incoming parallel data to DVI.

But I think it's better to remove the use of the word to avoid any
confusion. I'll make the change.

> > +	if (gpio_is_valid(ddata->pd_gpio)) {
> > +		r = gpio_request_one(ddata->pd_gpio, GPIOF_OUT_INIT_LOW,
> > +				"tfp410 pd");
> > +		if (r) {
> > +			dev_err(&dssdev->dev, "Failed to request PD GPIO %d\n",
> > +					ddata->pd_gpio);
> > +			ddata->pd_gpio = -1;
> 
> Is the power down gpio not a necessary thing? If it is, we should quit 
> here itself, shouldn't we?

Hmm, yes, I think you are right.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 7/7] OMAPDSS: HDMI: hot plug detect fix
From: Tomi Valkeinen @ 2012-03-08  7:35 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, linux-fbdev, linux-omap, Rob Clark
In-Reply-To: <20120307200146.GA26451@kroah.com>

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On Wed, 2012-03-07 at 12:01 -0800, Greg KH wrote:
> On Thu, Mar 01, 2012 at 02:26:35PM +0200, Tomi Valkeinen wrote:
> > From: Rob Clark <rob@ti.com>
> > 
> > The "OMAPDSS: HDMI: PHY burnout fix" commit switched the HDMI driver
> > over to using a GPIO for plug detect.  Unfortunately the ->detect()
> > method was not also updated, causing HDMI to no longer work for the
> > omapdrm driver (because it would actually check if a connection was
> > detected before attempting to enable display).
> > 
> > Signed-off-by: Rob Clark <rob@ti.com>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> You forgot to tell me what the git commit id is for this patch (it's
> ca888a7958b3d808e4efd08ceff88913f4212c69, right?)

Yes, that's the one. It wasn't in Linus's tree yet, only in fbdev tree,
so I wasn't sure what the commit id is.

> And why isn't this needed for the 3.0 kernel as well?

The detect() function is not present in 3.0, so there was nothing to
break.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [RFC PATCH] video:backlight: add dimming sysfs node
From: Donghwa Lee @ 2012-03-08  6:52 UTC (permalink / raw)
  To: linux-arm-kernel

In backlight class, update_status() callback function is mostly used to change
backlight brightness. When platform enter the dimming state, it is usually used.
But, I think dimming state can be defined variety of method including brightness.
So, it is need to differentiated node from brightness node.

In set_dimming() callback function, developers can define variety dimming functions.

Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/backlight/backlight.c |   37 +++++++++++++++++++++++++++++++++++
 include/linux/backlight.h           |    9 ++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bf5b1ec..44a77e4 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -101,6 +101,43 @@ static void backlight_generate_event(struct backlight_device *bd,
 	sysfs_notify(&bd->dev.kobj, NULL, "actual_brightness");
 }
 
+static ssize_t backlight_store_dimming(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	int rc;
+	struct backlight_device *bd = to_backlight_device(dev);
+	unsigned long dimming;
+
+	rc = strict_strtoul(buf, 0, &dimming);
+	if (rc)
+		return rc;
+
+	if (dimming < 0)
+		rc = -EINVAL;
+	else {
+		pr_debug("set dimming mode\n");
+
+		if (dimming)
+			bd->props.dimming = true;
+		else
+			bd->props.dimming = false;
+
+		backlight_set_dimming(bd);
+
+		rc = count;
+	}
+
+	return rc;
+}
+
+static ssize_t backlight_show_dimming(struct device *dev,
+		struct device_attribute *attr,char *buf)
+{
+	struct backlight_device *bd = to_backlight_device(dev);
+
+	return sprintf(buf, "%d\n", bd->props.dimming);
+}
+
 static ssize_t backlight_show_power(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 5ffc6dd..823717e 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -55,10 +55,13 @@ struct backlight_ops {
 	/* Check if given framebuffer device is the one bound to this backlight;
 	   return 0 if not, !=0 if it is. If NULL, backlight always matches the fb. */
 	int (*check_fb)(struct backlight_device *, struct fb_info *);
+	/* Notify the backlight driver to enter the dimming state */
+	int (*set_dimming)(struct backlight_device *);
 };
 
 /* This structure defines all the properties of a backlight */
 struct backlight_properties {
+	bool dimming;
 	/* Current User requested brightness (0 - max_brightness) */
 	int brightness;
 	/* Maximal value for brightness (read-only) */
@@ -111,6 +114,12 @@ static inline void backlight_update_status(struct backlight_device *bd)
 	mutex_unlock(&bd->update_lock);
 }
 
+static inline void backlight_set_dimming(struct backlight_device *bd)
+{
+	if (bd->ops && bd->ops->set_dimming)
+		bd->ops->set_dimming(bd);
+}
+
 extern struct backlight_device *backlight_device_register(const char *name,
 	struct device *dev, void *devdata, const struct backlight_ops *ops,
 	const struct backlight_properties *props);
-- 
1.7.4.1

^ permalink raw reply related

* RE: [GIT PULL] udlfb patches for fbdev-next
From: Jingoo Han @ 2012-03-08  0:12 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CAO1w=s86ykpLJC8KkTbUn1qhvO52CJnzCKz5+rhS7mQtnwR4_w@mail.gmail.com>

> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-owner@vger.kernel.org] On Behalf Of Bernie
> Thompson
> Sent: Thursday, March 08, 2012 3:40 AM
> To: Florian Tobias Schandinat
> Cc: Linux Fbdev development list
> Subject: Re: [GIT PULL] udlfb patches for fbdev-next
> 
> Hi Florian,
> 
> On Wed, Mar 7, 2012 at 2:06 AM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
> > Merged.
> 
> Thanks so much for doing that review/merge work.
> 
> > I was wondering about
> > the patch of Olivier Sobrie as it was neither part of this pull-request
> > nor did you comment on it, but probably you read my comment I wrote when
> > I cc'ed you as you added yourself to MAINTAINERS.
> 
> That patch looks good. I didn't include it only because I hadn't
> tested it.  Note that Jingoo's recommendation might run afoul of
> checkpatch.pl (but Olivier's 2nd version worked around that anyway, so
> it's fine - I'd recommend you take it).

Yes, you are right.
Also, Oliver's 2nd version patch looks good.
Good luck.

Best regards,
Jingoo Han

> 
> Thanks for your work maintaining fbdev.  Best wishes,
> Bernie
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox