diff mbox

video: OF display-timings support for ocfb

Message ID 1402517871-27503-1-git-send-email-franck.jullien@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

franck.jullien@gmail.com June 11, 2014, 8:17 p.m. UTC
Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
---
 drivers/video/fbdev/Kconfig |    2 ++
 drivers/video/fbdev/ocfb.c  |   31 ++++++++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

Comments

Stefan Kristiansson June 17, 2014, 9:08 p.m. UTC | #1
On Wed, Jun 11, 2014 at 10:17:51PM +0200, Franck Jullien wrote:
> Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
> ---
>  drivers/video/fbdev/Kconfig |    2 ++
>  drivers/video/fbdev/ocfb.c  |   31 ++++++++++++++++++++++++++-----
>  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index e1f4727..b4ac6bb 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -942,6 +942,8 @@ config FB_OPENCORES
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
>  	help
>  	  This enables support for the OpenCores VGA/LCD core.
>  
> diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
> index 7f9dc9b..6d15565 100644
> --- a/drivers/video/fbdev/ocfb.c
> +++ b/drivers/video/fbdev/ocfb.c
> @@ -22,6 +22,8 @@
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  
> +#include <video/of_display_timing.h>
> +
>  /* OCFB register defines */
>  #define OCFB_CTRL	0x000
>  #define OCFB_STAT	0x004
> @@ -310,11 +312,30 @@ static int ocfb_probe(struct platform_device *pdev)
>  	fbdev->info.device = &pdev->dev;
>  	fbdev->info.par = fbdev;
>  
> -	/* Video mode setup */
> -	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
> -			  NULL, 0, &default_mode, 16)) {
> -		dev_err(&pdev->dev, "No valid video modes found\n");
> -		return -EINVAL;
> +	if (!mode_option && IS_ENABLED(CONFIG_OF)) {
> +		struct fb_videomode mode;
> +		u32 bpp;
> +
> +		ret = of_get_fb_videomode(pdev->dev.of_node, &mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (ret)
> +			return ret;
> +
> +		fb_videomode_to_var(&fbdev->info.var, &mode);
> +
> +		ret = of_property_read_u32(pdev->dev.of_node, "bits-per-pixel",
> +					   &bpp);
> +		if (ret)
> +			return ret;
> +
> +		fbdev->info.var.bits_per_pixel = bpp;
> +
> +	} else {
> +		if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
> +				  NULL, 0, &default_mode, 16)) {
> +			dev_err(&pdev->dev, "No valid video modes found\n");
> +			return -EINVAL;
> +		}
>  	}
>  	ocfb_init_var(fbdev);
>  	ocfb_init_fix(fbdev);


Nice, this was something that was discussed as a possible future improvement
when I first posted this driver.

Acked-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
--
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
Tomi Valkeinen June 23, 2014, 12:01 p.m. UTC | #2
On 11/06/14 23:17, Franck Jullien wrote:
> Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
> ---
>  drivers/video/fbdev/Kconfig |    2 ++
>  drivers/video/fbdev/ocfb.c  |   31 ++++++++++++++++++++++++++-----
>  2 files changed, 28 insertions(+), 5 deletions(-)

You need to add binding documentation if you add new things to the
driver's DT support. Actually, we seem to be missing the the binding
documentation totally, even if the driver has 'of_device_id' table...

> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index e1f4727..b4ac6bb 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -942,6 +942,8 @@ config FB_OPENCORES
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
>  	help
>  	  This enables support for the OpenCores VGA/LCD core.
>  
> diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
> index 7f9dc9b..6d15565 100644
> --- a/drivers/video/fbdev/ocfb.c
> +++ b/drivers/video/fbdev/ocfb.c
> @@ -22,6 +22,8 @@
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  
> +#include <video/of_display_timing.h>
> +
>  /* OCFB register defines */
>  #define OCFB_CTRL	0x000
>  #define OCFB_STAT	0x004
> @@ -310,11 +312,30 @@ static int ocfb_probe(struct platform_device *pdev)
>  	fbdev->info.device = &pdev->dev;
>  	fbdev->info.par = fbdev;
>  
> -	/* Video mode setup */
> -	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
> -			  NULL, 0, &default_mode, 16)) {
> -		dev_err(&pdev->dev, "No valid video modes found\n");
> -		return -EINVAL;
> +	if (!mode_option && IS_ENABLED(CONFIG_OF)) {

This doesn't look correct. The kernel may have CONFIG_OF enabled, even
if the board does not use OF.

 Tomi
Stefan Kristiansson June 24, 2014, 6:59 a.m. UTC | #3
On Mon, Jun 23, 2014 at 03:01:36PM +0300, Tomi Valkeinen wrote:
> On 11/06/14 23:17, Franck Jullien wrote:
> > Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
> > ---
> >  drivers/video/fbdev/Kconfig |    2 ++
> >  drivers/video/fbdev/ocfb.c  |   31 ++++++++++++++++++++++++++-----
> >  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> You need to add binding documentation if you add new things to the
> driver's DT support. Actually, we seem to be missing the the binding
> documentation totally, even if the driver has 'of_device_id' table...
> 

Right, I'll take the blame for the missing driver binding documentation,
I'll write something up and post a patch for that.
But, where should it go?
Documentation/devicetree/bindings/video/
or
Documentation/devicetree/bindings/fb/

Stefan
--
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
diff mbox

Patch

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index e1f4727..b4ac6bb 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -942,6 +942,8 @@  config FB_OPENCORES
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	help
 	  This enables support for the OpenCores VGA/LCD core.
 
diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
index 7f9dc9b..6d15565 100644
--- a/drivers/video/fbdev/ocfb.c
+++ b/drivers/video/fbdev/ocfb.c
@@ -22,6 +22,8 @@ 
 #include <linux/string.h>
 #include <linux/slab.h>
 
+#include <video/of_display_timing.h>
+
 /* OCFB register defines */
 #define OCFB_CTRL	0x000
 #define OCFB_STAT	0x004
@@ -310,11 +312,30 @@  static int ocfb_probe(struct platform_device *pdev)
 	fbdev->info.device = &pdev->dev;
 	fbdev->info.par = fbdev;
 
-	/* Video mode setup */
-	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
-			  NULL, 0, &default_mode, 16)) {
-		dev_err(&pdev->dev, "No valid video modes found\n");
-		return -EINVAL;
+	if (!mode_option && IS_ENABLED(CONFIG_OF)) {
+		struct fb_videomode mode;
+		u32 bpp;
+
+		ret = of_get_fb_videomode(pdev->dev.of_node, &mode,
+					  OF_USE_NATIVE_MODE);
+		if (ret)
+			return ret;
+
+		fb_videomode_to_var(&fbdev->info.var, &mode);
+
+		ret = of_property_read_u32(pdev->dev.of_node, "bits-per-pixel",
+					   &bpp);
+		if (ret)
+			return ret;
+
+		fbdev->info.var.bits_per_pixel = bpp;
+
+	} else {
+		if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
+				  NULL, 0, &default_mode, 16)) {
+			dev_err(&pdev->dev, "No valid video modes found\n");
+			return -EINVAL;
+		}
 	}
 	ocfb_init_var(fbdev);
 	ocfb_init_fix(fbdev);