[PATCH v2 0/4] fb/drm: Add support for a panel-orientation connector prop

[PATCH v2 0/4] fb/drm: Add support for a panel-orientation connector prop

[Hans de Goede]

Hi All,

This is a resend of v2 of my panel-orientation drm connector property
series. I already send this out 2 weeks ago from my Red Hat email, but
that does not get picked up by the drm subsys scripts to run tests, so
hence this resend from my gmail.

The 2 biggest changes in v2 are:

1) Add support to drm-fb-helper.c for the panel-orientation prop and
make it apply the rotation to the fb to get fbcon to show the right way up.

2) Change the patch to enable the panel-orientation prop in the i915 driver
to read back to rotation setup by the BIOS from the hardware (the VBT
has a panel rotation field for this, but it reads 0 on hardware where
the GOP does correctly rotate the screen, so its useless).

Regards,

Hans

[PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Hans de Goede]

Some x86 clamshell design devices use portrait tablet LCD panels and a
display engine which cannot (transparently) rotate in hardware, so we
need to rotate things in software / let user space deal with this.

The fbcon code has a set of DMI based quirks to automatically detect
such tablet LCD panels and rotate the fbcon to compensate.

The plan was for userspace (e.g. a Wayland compositor) to simply read
/sys/class/graphics/fbcon/rotate and apply the rotation from there to
the LCD panel to compensate.

However this will not work when an external monitor gets plugged in,
since with fbcon rotation is not per output, so the fbcon quirk code
disables the rotation when an external monitor is present.

Using /sys/class/graphics/fbcon/rotate will not help in that case
since it will indicate no rotation is in use. So instead we are going
to need a drm connecter property for this.

This commit is a preparation patch for adding the drm-connecter
property, by making the fbcon quirk code generally usable so that
the drm code can use it to check for rotation quirks.

Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
if the quirk code should be build, since that is already selected by
both the drm and fbcon code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebased on 4.14-rc1
---
 drivers/video/fbdev/core/Makefile                  |  6 +++---
 .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
 drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
 drivers/video/fbdev/core/fbcon.h                   |  6 ------
 include/linux/fb.h                                 |  6 ++++++
 5 files changed, 32 insertions(+), 23 deletions(-)
 rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 73493bbd7a15..06caf037a822 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,7 @@
 obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
+ifeq ($(CONFIG_DMI),y)
+obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
+endif
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
@@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
 fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
 				     fbcon_ccw.o
 endif
-ifeq ($(CONFIG_DMI),y)
-fb-y				  += fbcon_dmi_quirks.o
-endif
 endif
 fb-objs                           := $(fb-y)
 
diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
similarity index 91%
rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
rename to drivers/video/fbdev/core/fb_dmi_quirks.c
index 6904e47d1e51..d5fdf3245f83 100644
--- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
+++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
@@ -1,5 +1,5 @@
 /*
- *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
+ *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
  *
  *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
  *
@@ -11,7 +11,6 @@
 #include <linux/dmi.h>
 #include <linux/fb.h>
 #include <linux/kernel.h>
-#include "fbcon.h"
 
 /*
  * Some x86 clamshell design devices use portrait tablet screens and a display
@@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
 	{}
 };
 
-int fbcon_platform_get_rotate(struct fb_info *info)
+/*
+ * Note this function returns the rotation necessary to put the display
+ * the right way up, or -1 if there is no quirk.
+ */
+int fb_get_panel_rotate_quirk(int width, int height)
 {
 	const struct dmi_system_id *match;
 	const struct fbcon_dmi_rotate_data *data;
@@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
 	     match = dmi_first_match(match + 1)) {
 		data = match->driver_data;
 
-		if (data->width != info->var.xres ||
-		    data->height != info->var.yres)
+		if (data->width != width || data->height != height)
 			continue;
 
 		if (!data->bios_dates)
@@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
 		}
 	}
 
-	return FB_ROTATE_UR;
+	return -1;
 }
+EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 04612f938bab..2e17ea02c295 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
 	ops->cur_rotate = -1;
 	ops->cur_blink_jiffies = HZ / 5;
 	info->fbcon_par = ops;
-	if (initial_rotation != -1)
-		p->con_rotate = initial_rotation;
-	else
-		p->con_rotate = fbcon_platform_get_rotate(info);
+	p->con_rotate = initial_rotation;
+	if (p->con_rotate = -1)
+		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
+							  info->var.yres);
+	if (p->con_rotate = -1)
+		p->con_rotate = FB_ROTATE_UR;
+
 	set_blitting_type(vc, info);
 
 	if (info->fix.type != FB_TYPE_TEXT) {
@@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
 
 	ops = info->fbcon_par;
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	if (initial_rotation != -1)
-		p->con_rotate = initial_rotation;
-	else
-		p->con_rotate = fbcon_platform_get_rotate(info);
+	p->con_rotate = initial_rotation;
+	if (p->con_rotate = -1)
+		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
+							  info->var.yres);
+	if (p->con_rotate = -1)
+		p->con_rotate = FB_ROTATE_UR;
+
 	set_blitting_type(vc, info);
 
 	cols = vc->vc_cols;
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 18f3ac144237..3746828356ed 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
 #define fbcon_set_rotate(x) do {} while(0)
 #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
 
-#ifdef CONFIG_DMI
-int fbcon_platform_get_rotate(struct fb_info *info);
-#else
-#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
-#endif /* CONFIG_DMI */
-
 #endif /* _VIDEO_FBCON_H */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f4386b0ccf40..7527965c5b53 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
 			const struct fb_videomode *default_mode,
 			unsigned int default_bpp);
 
+#ifdef CONFIG_DMI
+int fb_get_panel_rotate_quirk(int width, int height);
+#else
+#define fb_get_panel_rotate_quirk(width, height) (-1)
+#endif /* CONFIG_DMI */
+
 /* Convenience logging macros */
 #define fb_err(fb_info, fmt, ...)					\
 	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
-- 
2.14.2

Re: [PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Daniel Vetter]

On Sun, Oct 01, 2017 at 05:33:14PM +0200, Hans de Goede wrote:
> Some x86 clamshell design devices use portrait tablet LCD panels and a
> display engine which cannot (transparently) rotate in hardware, so we
> need to rotate things in software / let user space deal with this.
> 
> The fbcon code has a set of DMI based quirks to automatically detect
> such tablet LCD panels and rotate the fbcon to compensate.
> 
> The plan was for userspace (e.g. a Wayland compositor) to simply read
> /sys/class/graphics/fbcon/rotate and apply the rotation from there to
> the LCD panel to compensate.
> 
> However this will not work when an external monitor gets plugged in,
> since with fbcon rotation is not per output, so the fbcon quirk code
> disables the rotation when an external monitor is present.
> 
> Using /sys/class/graphics/fbcon/rotate will not help in that case
> since it will indicate no rotation is in use. So instead we are going
> to need a drm connecter property for this.
> 
> This commit is a preparation patch for adding the drm-connecter
> property, by making the fbcon quirk code generally usable so that
> the drm code can use it to check for rotation quirks.
> 
> Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
> if the quirk code should be build, since that is already selected by
> both the drm and fbcon code.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Can we pls just outright move this out of fbdev? fbdev is dead, I don't
want to add/maintain new stuff in there (except fbcon, but that should not
deal with stuff like this).

This probably means some serious patch series acrobatics, or just breaking
the current fbcon-only hack and rebuilding it in drm (in the same series).
-Daniel

> ---
> Changes in v2:
> -Rebased on 4.14-rc1
> ---
>  drivers/video/fbdev/core/Makefile                  |  6 +++---
>  .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
>  drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
>  drivers/video/fbdev/core/fbcon.h                   |  6 ------
>  include/linux/fb.h                                 |  6 ++++++
>  5 files changed, 32 insertions(+), 23 deletions(-)
>  rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)
> 
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index 73493bbd7a15..06caf037a822 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,4 +1,7 @@
>  obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
> +ifeq ($(CONFIG_DMI),y)
> +obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
> +endif
>  obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>  obj-$(CONFIG_FB)                  += fb.o
>  fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> @@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
>  fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
>  				     fbcon_ccw.o
>  endif
> -ifeq ($(CONFIG_DMI),y)
> -fb-y				  += fbcon_dmi_quirks.o
> -endif
>  endif
>  fb-objs                           := $(fb-y)
>  
> diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
> similarity index 91%
> rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
> rename to drivers/video/fbdev/core/fb_dmi_quirks.c
> index 6904e47d1e51..d5fdf3245f83 100644
> --- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
> +++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
> @@ -1,5 +1,5 @@
>  /*
> - *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
> + *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
>   *
>   *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
>   *
> @@ -11,7 +11,6 @@
>  #include <linux/dmi.h>
>  #include <linux/fb.h>
>  #include <linux/kernel.h>
> -#include "fbcon.h"
>  
>  /*
>   * Some x86 clamshell design devices use portrait tablet screens and a display
> @@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
>  	{}
>  };
>  
> -int fbcon_platform_get_rotate(struct fb_info *info)
> +/*
> + * Note this function returns the rotation necessary to put the display
> + * the right way up, or -1 if there is no quirk.
> + */
> +int fb_get_panel_rotate_quirk(int width, int height)
>  {
>  	const struct dmi_system_id *match;
>  	const struct fbcon_dmi_rotate_data *data;
> @@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>  	     match = dmi_first_match(match + 1)) {
>  		data = match->driver_data;
>  
> -		if (data->width != info->var.xres ||
> -		    data->height != info->var.yres)
> +		if (data->width != width || data->height != height)
>  			continue;
>  
>  		if (!data->bios_dates)
> @@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>  		}
>  	}
>  
> -	return FB_ROTATE_UR;
> +	return -1;
>  }
> +EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 04612f938bab..2e17ea02c295 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
>  	ops->cur_rotate = -1;
>  	ops->cur_blink_jiffies = HZ / 5;
>  	info->fbcon_par = ops;
> -	if (initial_rotation != -1)
> -		p->con_rotate = initial_rotation;
> -	else
> -		p->con_rotate = fbcon_platform_get_rotate(info);
> +	p->con_rotate = initial_rotation;
> +	if (p->con_rotate = -1)
> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
> +							  info->var.yres);
> +	if (p->con_rotate = -1)
> +		p->con_rotate = FB_ROTATE_UR;
> +
>  	set_blitting_type(vc, info);
>  
>  	if (info->fix.type != FB_TYPE_TEXT) {
> @@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>  
>  	ops = info->fbcon_par;
>  	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> -	if (initial_rotation != -1)
> -		p->con_rotate = initial_rotation;
> -	else
> -		p->con_rotate = fbcon_platform_get_rotate(info);
> +	p->con_rotate = initial_rotation;
> +	if (p->con_rotate = -1)
> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
> +							  info->var.yres);
> +	if (p->con_rotate = -1)
> +		p->con_rotate = FB_ROTATE_UR;
> +
>  	set_blitting_type(vc, info);
>  
>  	cols = vc->vc_cols;
> diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
> index 18f3ac144237..3746828356ed 100644
> --- a/drivers/video/fbdev/core/fbcon.h
> +++ b/drivers/video/fbdev/core/fbcon.h
> @@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
>  #define fbcon_set_rotate(x) do {} while(0)
>  #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
>  
> -#ifdef CONFIG_DMI
> -int fbcon_platform_get_rotate(struct fb_info *info);
> -#else
> -#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
> -#endif /* CONFIG_DMI */
> -
>  #endif /* _VIDEO_FBCON_H */
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index f4386b0ccf40..7527965c5b53 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
>  			const struct fb_videomode *default_mode,
>  			unsigned int default_bpp);
>  
> +#ifdef CONFIG_DMI
> +int fb_get_panel_rotate_quirk(int width, int height);
> +#else
> +#define fb_get_panel_rotate_quirk(width, height) (-1)
> +#endif /* CONFIG_DMI */
> +
>  /* Convenience logging macros */
>  #define fb_err(fb_info, fmt, ...)					\
>  	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Hans de Goede]

Hi,

Sorry for being slow to reply, I've been a bit overwhelmed with
other stuff lately.

On 10/02/2017 10:01 AM, Daniel Vetter wrote:
> On Sun, Oct 01, 2017 at 05:33:14PM +0200, Hans de Goede wrote:
>> Some x86 clamshell design devices use portrait tablet LCD panels and a
>> display engine which cannot (transparently) rotate in hardware, so we
>> need to rotate things in software / let user space deal with this.
>>
>> The fbcon code has a set of DMI based quirks to automatically detect
>> such tablet LCD panels and rotate the fbcon to compensate.
>>
>> The plan was for userspace (e.g. a Wayland compositor) to simply read
>> /sys/class/graphics/fbcon/rotate and apply the rotation from there to
>> the LCD panel to compensate.
>>
>> However this will not work when an external monitor gets plugged in,
>> since with fbcon rotation is not per output, so the fbcon quirk code
>> disables the rotation when an external monitor is present.
>>
>> Using /sys/class/graphics/fbcon/rotate will not help in that case
>> since it will indicate no rotation is in use. So instead we are going
>> to need a drm connecter property for this.
>>
>> This commit is a preparation patch for adding the drm-connecter
>> property, by making the fbcon quirk code generally usable so that
>> the drm code can use it to check for rotation quirks.
>>
>> Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
>> if the quirk code should be build, since that is already selected by
>> both the drm and fbcon code.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Can we pls just outright move this out of fbdev? fbdev is dead, I don't
> want to add/maintain new stuff in there (except fbcon, but that should not
> deal with stuff like this).

Sure we can do that, but fbcon also needs access to the quirks, so then
we need to have some part of the drm code which always gets builtin into
the kernel and make the drm code export a function for this which the
fbcon code uses.

The reason why fbcon uses this is because there are cases where the BIOS
does not setup the EFIFB with the correct rotation, typically because
the hardware does not support 90 / 270 degrees rotation and the
screen has been mounted rotated 90 / 270 degrees rotation.

Regards,

Hans


> This probably means some serious patch series acrobatics, or just breaking
> the current fbcon-only hack and rebuilding it in drm (in the same series).
 >
> -Daniel
> 
>> ---
>> Changes in v2:
>> -Rebased on 4.14-rc1
>> ---
>>   drivers/video/fbdev/core/Makefile                  |  6 +++---
>>   .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
>>   drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
>>   drivers/video/fbdev/core/fbcon.h                   |  6 ------
>>   include/linux/fb.h                                 |  6 ++++++
>>   5 files changed, 32 insertions(+), 23 deletions(-)
>>   rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)
>>
>> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
>> index 73493bbd7a15..06caf037a822 100644
>> --- a/drivers/video/fbdev/core/Makefile
>> +++ b/drivers/video/fbdev/core/Makefile
>> @@ -1,4 +1,7 @@
>>   obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
>> +ifeq ($(CONFIG_DMI),y)
>> +obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
>> +endif
>>   obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>>   obj-$(CONFIG_FB)                  += fb.o
>>   fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
>> @@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
>>   fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
>>   				     fbcon_ccw.o
>>   endif
>> -ifeq ($(CONFIG_DMI),y)
>> -fb-y				  += fbcon_dmi_quirks.o
>> -endif
>>   endif
>>   fb-objs                           := $(fb-y)
>>   
>> diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
>> similarity index 91%
>> rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
>> rename to drivers/video/fbdev/core/fb_dmi_quirks.c
>> index 6904e47d1e51..d5fdf3245f83 100644
>> --- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
>> +++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
>> @@ -1,5 +1,5 @@
>>   /*
>> - *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
>> + *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
>>    *
>>    *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
>>    *
>> @@ -11,7 +11,6 @@
>>   #include <linux/dmi.h>
>>   #include <linux/fb.h>
>>   #include <linux/kernel.h>
>> -#include "fbcon.h"
>>   
>>   /*
>>    * Some x86 clamshell design devices use portrait tablet screens and a display
>> @@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
>>   	{}
>>   };
>>   
>> -int fbcon_platform_get_rotate(struct fb_info *info)
>> +/*
>> + * Note this function returns the rotation necessary to put the display
>> + * the right way up, or -1 if there is no quirk.
>> + */
>> +int fb_get_panel_rotate_quirk(int width, int height)
>>   {
>>   	const struct dmi_system_id *match;
>>   	const struct fbcon_dmi_rotate_data *data;
>> @@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>>   	     match = dmi_first_match(match + 1)) {
>>   		data = match->driver_data;
>>   
>> -		if (data->width != info->var.xres ||
>> -		    data->height != info->var.yres)
>> +		if (data->width != width || data->height != height)
>>   			continue;
>>   
>>   		if (!data->bios_dates)
>> @@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>>   		}
>>   	}
>>   
>> -	return FB_ROTATE_UR;
>> +	return -1;
>>   }
>> +EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>> index 04612f938bab..2e17ea02c295 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
>>   	ops->cur_rotate = -1;
>>   	ops->cur_blink_jiffies = HZ / 5;
>>   	info->fbcon_par = ops;
>> -	if (initial_rotation != -1)
>> -		p->con_rotate = initial_rotation;
>> -	else
>> -		p->con_rotate = fbcon_platform_get_rotate(info);
>> +	p->con_rotate = initial_rotation;
>> +	if (p->con_rotate = -1)
>> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
>> +							  info->var.yres);
>> +	if (p->con_rotate = -1)
>> +		p->con_rotate = FB_ROTATE_UR;
>> +
>>   	set_blitting_type(vc, info);
>>   
>>   	if (info->fix.type != FB_TYPE_TEXT) {
>> @@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>>   
>>   	ops = info->fbcon_par;
>>   	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
>> -	if (initial_rotation != -1)
>> -		p->con_rotate = initial_rotation;
>> -	else
>> -		p->con_rotate = fbcon_platform_get_rotate(info);
>> +	p->con_rotate = initial_rotation;
>> +	if (p->con_rotate = -1)
>> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
>> +							  info->var.yres);
>> +	if (p->con_rotate = -1)
>> +		p->con_rotate = FB_ROTATE_UR;
>> +
>>   	set_blitting_type(vc, info);
>>   
>>   	cols = vc->vc_cols;
>> diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
>> index 18f3ac144237..3746828356ed 100644
>> --- a/drivers/video/fbdev/core/fbcon.h
>> +++ b/drivers/video/fbdev/core/fbcon.h
>> @@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
>>   #define fbcon_set_rotate(x) do {} while(0)
>>   #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
>>   
>> -#ifdef CONFIG_DMI
>> -int fbcon_platform_get_rotate(struct fb_info *info);
>> -#else
>> -#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
>> -#endif /* CONFIG_DMI */
>> -
>>   #endif /* _VIDEO_FBCON_H */
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index f4386b0ccf40..7527965c5b53 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
>>   			const struct fb_videomode *default_mode,
>>   			unsigned int default_bpp);
>>   
>> +#ifdef CONFIG_DMI
>> +int fb_get_panel_rotate_quirk(int width, int height);
>> +#else
>> +#define fb_get_panel_rotate_quirk(width, height) (-1)
>> +#endif /* CONFIG_DMI */
>> +
>>   /* Convenience logging macros */
>>   #define fb_err(fb_info, fmt, ...)					\
>>   	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
>> -- 
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Daniel Vetter]

On Tue, Oct 17, 2017 at 06:17:21PM +0200, Hans de Goede wrote:
> Hi,
> 
> Sorry for being slow to reply, I've been a bit overwhelmed with
> other stuff lately.
> 
> On 10/02/2017 10:01 AM, Daniel Vetter wrote:
> > On Sun, Oct 01, 2017 at 05:33:14PM +0200, Hans de Goede wrote:
> > > Some x86 clamshell design devices use portrait tablet LCD panels and a
> > > display engine which cannot (transparently) rotate in hardware, so we
> > > need to rotate things in software / let user space deal with this.
> > > 
> > > The fbcon code has a set of DMI based quirks to automatically detect
> > > such tablet LCD panels and rotate the fbcon to compensate.
> > > 
> > > The plan was for userspace (e.g. a Wayland compositor) to simply read
> > > /sys/class/graphics/fbcon/rotate and apply the rotation from there to
> > > the LCD panel to compensate.
> > > 
> > > However this will not work when an external monitor gets plugged in,
> > > since with fbcon rotation is not per output, so the fbcon quirk code
> > > disables the rotation when an external monitor is present.
> > > 
> > > Using /sys/class/graphics/fbcon/rotate will not help in that case
> > > since it will indicate no rotation is in use. So instead we are going
> > > to need a drm connecter property for this.
> > > 
> > > This commit is a preparation patch for adding the drm-connecter
> > > property, by making the fbcon quirk code generally usable so that
> > > the drm code can use it to check for rotation quirks.
> > > 
> > > Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
> > > if the quirk code should be build, since that is already selected by
> > > both the drm and fbcon code.
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Can we pls just outright move this out of fbdev? fbdev is dead, I don't
> > want to add/maintain new stuff in there (except fbcon, but that should not
> > deal with stuff like this).
> 
> Sure we can do that, but fbcon also needs access to the quirks, so then
> we need to have some part of the drm code which always gets builtin into
> the kernel and make the drm code export a function for this which the
> fbcon code uses.
> 
> The reason why fbcon uses this is because there are cases where the BIOS
> does not setup the EFIFB with the correct rotation, typically because
> the hardware does not support 90 / 270 degrees rotation and the
> screen has been mounted rotated 90 / 270 degrees rotation.

Oh dear, I hoped we could ignore that.

So yeah we need to load the same quirk table for efifb and for i915. How
much I wish we'd just have merged simpledrm by now :-)

I still don't think that fbcon should have access to the quirks directly,
but efifb and drm_fb_helper should set the same hint flags probably.

Wrt the location, I don't even think we need to have this built-in. Put a
new module for intel-quirks into drivers/gpu/platform and select that from
both, and it should all work out I think.

Or is there some other gotcha?

Thanks, Daniel

> 
> Regards,
> 
> Hans
> 
> 
> > This probably means some serious patch series acrobatics, or just breaking
> > the current fbcon-only hack and rebuilding it in drm (in the same series).
> >
> > -Daniel
> > 
> > > ---
> > > Changes in v2:
> > > -Rebased on 4.14-rc1
> > > ---
> > >   drivers/video/fbdev/core/Makefile                  |  6 +++---
> > >   .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
> > >   drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
> > >   drivers/video/fbdev/core/fbcon.h                   |  6 ------
> > >   include/linux/fb.h                                 |  6 ++++++
> > >   5 files changed, 32 insertions(+), 23 deletions(-)
> > >   rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)
> > > 
> > > diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> > > index 73493bbd7a15..06caf037a822 100644
> > > --- a/drivers/video/fbdev/core/Makefile
> > > +++ b/drivers/video/fbdev/core/Makefile
> > > @@ -1,4 +1,7 @@
> > >   obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
> > > +ifeq ($(CONFIG_DMI),y)
> > > +obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
> > > +endif
> > >   obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
> > >   obj-$(CONFIG_FB)                  += fb.o
> > >   fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> > > @@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
> > >   fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
> > >   				     fbcon_ccw.o
> > >   endif
> > > -ifeq ($(CONFIG_DMI),y)
> > > -fb-y				  += fbcon_dmi_quirks.o
> > > -endif
> > >   endif
> > >   fb-objs                           := $(fb-y)
> > > diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
> > > similarity index 91%
> > > rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
> > > rename to drivers/video/fbdev/core/fb_dmi_quirks.c
> > > index 6904e47d1e51..d5fdf3245f83 100644
> > > --- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
> > > +++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
> > > @@ -1,5 +1,5 @@
> > >   /*
> > > - *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
> > > + *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
> > >    *
> > >    *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> > >    *
> > > @@ -11,7 +11,6 @@
> > >   #include <linux/dmi.h>
> > >   #include <linux/fb.h>
> > >   #include <linux/kernel.h>
> > > -#include "fbcon.h"
> > >   /*
> > >    * Some x86 clamshell design devices use portrait tablet screens and a display
> > > @@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
> > >   	{}
> > >   };
> > > -int fbcon_platform_get_rotate(struct fb_info *info)
> > > +/*
> > > + * Note this function returns the rotation necessary to put the display
> > > + * the right way up, or -1 if there is no quirk.
> > > + */
> > > +int fb_get_panel_rotate_quirk(int width, int height)
> > >   {
> > >   	const struct dmi_system_id *match;
> > >   	const struct fbcon_dmi_rotate_data *data;
> > > @@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
> > >   	     match = dmi_first_match(match + 1)) {
> > >   		data = match->driver_data;
> > > -		if (data->width != info->var.xres ||
> > > -		    data->height != info->var.yres)
> > > +		if (data->width != width || data->height != height)
> > >   			continue;
> > >   		if (!data->bios_dates)
> > > @@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
> > >   		}
> > >   	}
> > > -	return FB_ROTATE_UR;
> > > +	return -1;
> > >   }
> > > +EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
> > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > > index 04612f938bab..2e17ea02c295 100644
> > > --- a/drivers/video/fbdev/core/fbcon.c
> > > +++ b/drivers/video/fbdev/core/fbcon.c
> > > @@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
> > >   	ops->cur_rotate = -1;
> > >   	ops->cur_blink_jiffies = HZ / 5;
> > >   	info->fbcon_par = ops;
> > > -	if (initial_rotation != -1)
> > > -		p->con_rotate = initial_rotation;
> > > -	else
> > > -		p->con_rotate = fbcon_platform_get_rotate(info);
> > > +	p->con_rotate = initial_rotation;
> > > +	if (p->con_rotate = -1)
> > > +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
> > > +							  info->var.yres);
> > > +	if (p->con_rotate = -1)
> > > +		p->con_rotate = FB_ROTATE_UR;
> > > +
> > >   	set_blitting_type(vc, info);
> > >   	if (info->fix.type != FB_TYPE_TEXT) {
> > > @@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
> > >   	ops = info->fbcon_par;
> > >   	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -	if (initial_rotation != -1)
> > > -		p->con_rotate = initial_rotation;
> > > -	else
> > > -		p->con_rotate = fbcon_platform_get_rotate(info);
> > > +	p->con_rotate = initial_rotation;
> > > +	if (p->con_rotate = -1)
> > > +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
> > > +							  info->var.yres);
> > > +	if (p->con_rotate = -1)
> > > +		p->con_rotate = FB_ROTATE_UR;
> > > +
> > >   	set_blitting_type(vc, info);
> > >   	cols = vc->vc_cols;
> > > diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
> > > index 18f3ac144237..3746828356ed 100644
> > > --- a/drivers/video/fbdev/core/fbcon.h
> > > +++ b/drivers/video/fbdev/core/fbcon.h
> > > @@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
> > >   #define fbcon_set_rotate(x) do {} while(0)
> > >   #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
> > > -#ifdef CONFIG_DMI
> > > -int fbcon_platform_get_rotate(struct fb_info *info);
> > > -#else
> > > -#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
> > > -#endif /* CONFIG_DMI */
> > > -
> > >   #endif /* _VIDEO_FBCON_H */
> > > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > > index f4386b0ccf40..7527965c5b53 100644
> > > --- a/include/linux/fb.h
> > > +++ b/include/linux/fb.h
> > > @@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
> > >   			const struct fb_videomode *default_mode,
> > >   			unsigned int default_bpp);
> > > +#ifdef CONFIG_DMI
> > > +int fb_get_panel_rotate_quirk(int width, int height);
> > > +#else
> > > +#define fb_get_panel_rotate_quirk(width, height) (-1)
> > > +#endif /* CONFIG_DMI */
> > > +
> > >   /* Convenience logging macros */
> > >   #define fb_err(fb_info, fmt, ...)					\
> > >   	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
> > > -- 
> > > 2.14.2
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Hans de Goede]

Hi,

On 18-10-17 10:03, Daniel Vetter wrote:
> On Tue, Oct 17, 2017 at 06:17:21PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> Sorry for being slow to reply, I've been a bit overwhelmed with
>> other stuff lately.
>>
>> On 10/02/2017 10:01 AM, Daniel Vetter wrote:
>>> On Sun, Oct 01, 2017 at 05:33:14PM +0200, Hans de Goede wrote:
>>>> Some x86 clamshell design devices use portrait tablet LCD panels and a
>>>> display engine which cannot (transparently) rotate in hardware, so we
>>>> need to rotate things in software / let user space deal with this.
>>>>
>>>> The fbcon code has a set of DMI based quirks to automatically detect
>>>> such tablet LCD panels and rotate the fbcon to compensate.
>>>>
>>>> The plan was for userspace (e.g. a Wayland compositor) to simply read
>>>> /sys/class/graphics/fbcon/rotate and apply the rotation from there to
>>>> the LCD panel to compensate.
>>>>
>>>> However this will not work when an external monitor gets plugged in,
>>>> since with fbcon rotation is not per output, so the fbcon quirk code
>>>> disables the rotation when an external monitor is present.
>>>>
>>>> Using /sys/class/graphics/fbcon/rotate will not help in that case
>>>> since it will indicate no rotation is in use. So instead we are going
>>>> to need a drm connecter property for this.
>>>>
>>>> This commit is a preparation patch for adding the drm-connecter
>>>> property, by making the fbcon quirk code generally usable so that
>>>> the drm code can use it to check for rotation quirks.
>>>>
>>>> Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
>>>> if the quirk code should be build, since that is already selected by
>>>> both the drm and fbcon code.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>> Can we pls just outright move this out of fbdev? fbdev is dead, I don't
>>> want to add/maintain new stuff in there (except fbcon, but that should not
>>> deal with stuff like this).
>>
>> Sure we can do that, but fbcon also needs access to the quirks, so then
>> we need to have some part of the drm code which always gets builtin into
>> the kernel and make the drm code export a function for this which the
>> fbcon code uses.
>>
>> The reason why fbcon uses this is because there are cases where the BIOS
>> does not setup the EFIFB with the correct rotation, typically because
>> the hardware does not support 90 / 270 degrees rotation and the
>> screen has been mounted rotated 90 / 270 degrees rotation.
> 
> Oh dear, I hoped we could ignore that.
> 
> So yeah we need to load the same quirk table for efifb and for i915. How
> much I wish we'd just have merged simpledrm by now :-)
> 
> I still don't think that fbcon should have access to the quirks directly,
> but efifb and drm_fb_helper should set the same hint flags probably.
> 
> Wrt the location, I don't even think we need to have this built-in. Put a
> new module for intel-quirks into drivers/gpu/platform and select that from
> both, and it should all work out I think.
> 
> Or is there some other gotcha?

Not that I can see, this all sounds reasonable to me. I will start reworking
the patch-set to address this (and your other comments) and post a new
version of the patch-set when I'm done.

Regards,

Hans




>>> This probably means some serious patch series acrobatics, or just breaking
>>> the current fbcon-only hack and rebuilding it in drm (in the same series).
>>>
>>> -Daniel
>>>
>>>> ---
>>>> Changes in v2:
>>>> -Rebased on 4.14-rc1
>>>> ---
>>>>    drivers/video/fbdev/core/Makefile                  |  6 +++---
>>>>    .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
>>>>    drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
>>>>    drivers/video/fbdev/core/fbcon.h                   |  6 ------
>>>>    include/linux/fb.h                                 |  6 ++++++
>>>>    5 files changed, 32 insertions(+), 23 deletions(-)
>>>>    rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)
>>>>
>>>> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
>>>> index 73493bbd7a15..06caf037a822 100644
>>>> --- a/drivers/video/fbdev/core/Makefile
>>>> +++ b/drivers/video/fbdev/core/Makefile
>>>> @@ -1,4 +1,7 @@
>>>>    obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
>>>> +ifeq ($(CONFIG_DMI),y)
>>>> +obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
>>>> +endif
>>>>    obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>>>>    obj-$(CONFIG_FB)                  += fb.o
>>>>    fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
>>>> @@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
>>>>    fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
>>>>    				     fbcon_ccw.o
>>>>    endif
>>>> -ifeq ($(CONFIG_DMI),y)
>>>> -fb-y				  += fbcon_dmi_quirks.o
>>>> -endif
>>>>    endif
>>>>    fb-objs                           := $(fb-y)
>>>> diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
>>>> similarity index 91%
>>>> rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
>>>> rename to drivers/video/fbdev/core/fb_dmi_quirks.c
>>>> index 6904e47d1e51..d5fdf3245f83 100644
>>>> --- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
>>>> +++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
>>>> @@ -1,5 +1,5 @@
>>>>    /*
>>>> - *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
>>>> + *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
>>>>     *
>>>>     *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
>>>>     *
>>>> @@ -11,7 +11,6 @@
>>>>    #include <linux/dmi.h>
>>>>    #include <linux/fb.h>
>>>>    #include <linux/kernel.h>
>>>> -#include "fbcon.h"
>>>>    /*
>>>>     * Some x86 clamshell design devices use portrait tablet screens and a display
>>>> @@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
>>>>    	{}
>>>>    };
>>>> -int fbcon_platform_get_rotate(struct fb_info *info)
>>>> +/*
>>>> + * Note this function returns the rotation necessary to put the display
>>>> + * the right way up, or -1 if there is no quirk.
>>>> + */
>>>> +int fb_get_panel_rotate_quirk(int width, int height)
>>>>    {
>>>>    	const struct dmi_system_id *match;
>>>>    	const struct fbcon_dmi_rotate_data *data;
>>>> @@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>>>>    	     match = dmi_first_match(match + 1)) {
>>>>    		data = match->driver_data;
>>>> -		if (data->width != info->var.xres ||
>>>> -		    data->height != info->var.yres)
>>>> +		if (data->width != width || data->height != height)
>>>>    			continue;
>>>>    		if (!data->bios_dates)
>>>> @@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
>>>>    		}
>>>>    	}
>>>> -	return FB_ROTATE_UR;
>>>> +	return -1;
>>>>    }
>>>> +EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
>>>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>>>> index 04612f938bab..2e17ea02c295 100644
>>>> --- a/drivers/video/fbdev/core/fbcon.c
>>>> +++ b/drivers/video/fbdev/core/fbcon.c
>>>> @@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
>>>>    	ops->cur_rotate = -1;
>>>>    	ops->cur_blink_jiffies = HZ / 5;
>>>>    	info->fbcon_par = ops;
>>>> -	if (initial_rotation != -1)
>>>> -		p->con_rotate = initial_rotation;
>>>> -	else
>>>> -		p->con_rotate = fbcon_platform_get_rotate(info);
>>>> +	p->con_rotate = initial_rotation;
>>>> +	if (p->con_rotate = -1)
>>>> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
>>>> +							  info->var.yres);
>>>> +	if (p->con_rotate = -1)
>>>> +		p->con_rotate = FB_ROTATE_UR;
>>>> +
>>>>    	set_blitting_type(vc, info);
>>>>    	if (info->fix.type != FB_TYPE_TEXT) {
>>>> @@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
>>>>    	ops = info->fbcon_par;
>>>>    	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
>>>> -	if (initial_rotation != -1)
>>>> -		p->con_rotate = initial_rotation;
>>>> -	else
>>>> -		p->con_rotate = fbcon_platform_get_rotate(info);
>>>> +	p->con_rotate = initial_rotation;
>>>> +	if (p->con_rotate = -1)
>>>> +		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
>>>> +							  info->var.yres);
>>>> +	if (p->con_rotate = -1)
>>>> +		p->con_rotate = FB_ROTATE_UR;
>>>> +
>>>>    	set_blitting_type(vc, info);
>>>>    	cols = vc->vc_cols;
>>>> diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
>>>> index 18f3ac144237..3746828356ed 100644
>>>> --- a/drivers/video/fbdev/core/fbcon.h
>>>> +++ b/drivers/video/fbdev/core/fbcon.h
>>>> @@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
>>>>    #define fbcon_set_rotate(x) do {} while(0)
>>>>    #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
>>>> -#ifdef CONFIG_DMI
>>>> -int fbcon_platform_get_rotate(struct fb_info *info);
>>>> -#else
>>>> -#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
>>>> -#endif /* CONFIG_DMI */
>>>> -
>>>>    #endif /* _VIDEO_FBCON_H */
>>>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>>>> index f4386b0ccf40..7527965c5b53 100644
>>>> --- a/include/linux/fb.h
>>>> +++ b/include/linux/fb.h
>>>> @@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
>>>>    			const struct fb_videomode *default_mode,
>>>>    			unsigned int default_bpp);
>>>> +#ifdef CONFIG_DMI
>>>> +int fb_get_panel_rotate_quirk(int width, int height);
>>>> +#else
>>>> +#define fb_get_panel_rotate_quirk(width, height) (-1)
>>>> +#endif /* CONFIG_DMI */
>>>> +
>>>>    /* Convenience logging macros */
>>>>    #define fb_err(fb_info, fmt, ...)					\
>>>>    	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
>>>> -- 
>>>> 2.14.2
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
> 

[PATCH v2 2/4] drm: Add support for a panel-orientation connector property

[Hans de Goede]

On some devices the LCD panel is mounted in the casing in such a way that
the up/top side of the panel does not match with the top side of the
device (e.g. it is mounted upside-down).

This commit adds the necessary infra for lcd-panel drm_connector-s to
have a "panel orientation" property to communicate how the panel is
orientated vs the casing.

Userspace can use this property to check for non-normal orientation and
then adjust the displayed image accordingly by rotating it to compensate.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebased on 4.14-rc1
-Store panel_orientation in drm_display_info, so that drm_fb_helper.c can
 access it easily
-Have a single drm_connector_init_panel_orientation_property rather then
 create and attach functions. The caller is expected to set
 drm_display_info.panel_orientation before calling this, then this will
 check for platform specific quirks overriding the panel_orientation and if
 the panel_orientation is set after this then it will attach the property.
---
 drivers/gpu/drm/drm_connector.c | 86 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h     | 11 ++++++
 include/drm/drm_mode_config.h   |  7 ++++
 include/uapi/drm/drm_mode.h     |  7 ++++
 4 files changed, 111 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ba9f36cef68c..0159800b30a7 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -212,6 +212,8 @@ int drm_connector_init(struct drm_device *dev,
 	mutex_init(&connector->mutex);
 	connector->edid_blob_ptr = NULL;
 	connector->status = connector_status_unknown;
+	connector->display_info.panel_orientation +		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
 
 	drm_connector_get_cmdline_mode(connector);
 
@@ -665,6 +667,13 @@ static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = {
 	{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
 };
 
+static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
+	{ DRM_MODE_PANEL_ORIENTATION_NORMAL,	"Normal"	},
+	{ DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP,	"Upside Down"	},
+	{ DRM_MODE_PANEL_ORIENTATION_LEFT_UP,	"Left Side Up"	},
+	{ DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,	"Right Side Up"	},
+};
+
 static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = {
 	{ DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
 	{ DRM_MODE_SUBCONNECTOR_DVID,      "DVI-D"     }, /* DVI-I  */
@@ -746,6 +755,18 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
  *
  * CRTC_ID:
  * 	Mode object ID of the &drm_crtc this connector should be connected to.
+ *
+ * Connectors for LCD panels may also have one standardized property:
+ *
+ * panel orientation:
+ *	On some devices the LCD panel is mounted in the casing in such a way
+ *	that the up/top side of the panel does not match with the top side of
+ *	the device. Userspace can use this property to check for this.
+ *	Note that input coordinates from touchscreens (input devices with
+ *	INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
+ *	coordinates, so if userspace rotates the picture to adjust for
+ *	the orientation it must also apply the same transformation to the
+ *	touchscreen input coordinates.
  */
 
 int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1212,6 +1233,71 @@ void drm_mode_connector_set_link_status_property(struct drm_connector *connector
 }
 EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
 
+/**
+ * drm_connector_init_panel_orientation_property -
+ *	initialize the connecters panel_orientation property
+ * @connector: connector for which to init the panel-orientation property.
+ * @width: width in pixels of the panel, used for panel quirk detection
+ * @height: height in pixels of the panel, used for panel quirk detection
+ *
+ * This function should only be called for built-in panels, after setting
+ * connector->display_info.panel_orientation first (if known).
+ *
+ * This function will check for platform specific (e.g. DMI based) quirks
+ * overriding display_info.panel_orientation first, then if panel_orientation
+ * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
+ * "panel orientation" property to the connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_init_panel_orientation_property(
+	struct drm_connector *connector, int width, int height)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_display_info *info = &connector->display_info;
+	struct drm_property *prop;
+
+	/*
+	 * Note fb_get_panel_rotate_quirk returns the rotation needed to
+	 * *correct* for the panel orientation.
+	 */
+	switch (fb_get_panel_rotate_quirk(width, height)) {
+	case FB_ROTATE_UR:
+		info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+		break;
+	case FB_ROTATE_CW:
+		info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+		break;
+	case FB_ROTATE_UD:
+		info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+		break;
+	case FB_ROTATE_CCW:
+		info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+		break;
+	}
+
+	if (info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return 0;
+
+	prop = dev->mode_config.panel_orientation_property;
+	if (!prop) {
+		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
+				"panel orientation",
+				drm_panel_orientation_enum_list,
+				ARRAY_SIZE(drm_panel_orientation_enum_list));
+		if (!prop)
+			return -ENOMEM;
+
+		dev->mode_config.panel_orientation_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop,
+				   info->panel_orientation);
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
+
 int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
 				    struct drm_property *property,
 				    uint64_t value)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ea8da401c93c..04592f9daa46 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -222,6 +222,15 @@ struct drm_display_info {
 #define DRM_COLOR_FORMAT_YCRCB422	(1<<2)
 #define DRM_COLOR_FORMAT_YCRCB420	(1<<3)
 
+	/**
+	 * @panel_orientation: Read only connector property for built-in panels,
+	 * indicating the orientation of the panel vs the device's casing.
+	 * drm_connector_init() sets this to DRM_MODE_PANEL_ORIENTATION_UNKNOWN.
+	 * When not UNKNOWN this gets used by the drm_fb_helpers to rotate the
+	 * fb to compensate and gets exported as prop to userspace.
+	 */
+	int panel_orientation;
+
 	/**
 	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
 	 * modes. Used DRM_COLOR_FORMAT\_ defines, which are _not_ the same ones
@@ -1012,6 +1021,8 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 					    const struct edid *edid);
 void drm_mode_connector_set_link_status_property(struct drm_connector *connector,
 						 uint64_t link_status);
+int drm_connector_init_panel_orientation_property(
+	struct drm_connector *connector, int width, int height);
 
 /**
  * struct drm_tile_group - Tile group metadata
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..6db187e4c747 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -741,6 +741,13 @@ struct drm_mode_config {
 	 */
 	struct drm_property *suggested_y_property;
 
+	/**
+	 * @panel_orientation_property: Optional connector property indicating
+	 * how the lcd-panel is mounted inside the casing (e.g. normal or
+	 * upside-down).
+	 */
+	struct drm_property *panel_orientation_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 54fc38c3c3f1..1a5dae7ba01c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -127,6 +127,13 @@ extern "C" {
 #define DRM_MODE_LINK_STATUS_GOOD	0
 #define DRM_MODE_LINK_STATUS_BAD	1
 
+/* Panel Orientation options */
+#define DRM_MODE_PANEL_ORIENTATION_UNKNOWN	-1
+#define DRM_MODE_PANEL_ORIENTATION_NORMAL	0
+#define DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP	1
+#define DRM_MODE_PANEL_ORIENTATION_LEFT_UP	2
+#define DRM_MODE_PANEL_ORIENTATION_RIGHT_UP	3
+
 /*
  * DRM_MODE_ROTATE_<degrees>
  *
-- 
2.14.2

[PATCH v2 3/4] drm/i915: Add "panel orientation" property to the panel connector

[Hans de Goede]

Ideally we could use the VBT for this, that would be simple, in
intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
connector->display_info.panel_orientation accordingly and call
drm_connector_init_panel_orientation_property(), done.

Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
with an upside down LCD and where the GOP is properly rotating the
EFI fb in hardware.

So instead we end up reading the rotation from the primary plane,
which is a bit more complicated.

The code to read back the rotation from the hardware is based on an
earlier attempt to fix fdo#94894 by Ville Syrjala.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebased on 4.14-rc1
-Readback primary plane rotation from the hardware and use that to
 set the panel orientation
-This (readback) fixes fdo#94894, add Fixes: tag
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
 drivers/gpu/drm/i915/intel_panel.c   | 33 ++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 18d9da53282b..c4c8590972b4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2151,6 +2151,8 @@ struct intel_cdclk_state {
 	unsigned int cdclk, vco, ref;
 };
 
+struct intel_panel;
+
 struct drm_i915_private {
 	struct drm_device drm;
 
@@ -2200,6 +2202,7 @@ struct drm_i915_private {
 	struct i915_gem_context *kernel_context;
 	struct intel_engine_cs *engine[I915_NUM_ENGINES];
 	struct i915_vma *semaphore;
+	struct intel_panel *panel;
 
 	struct drm_dma_handle *status_page_dmah;
 	struct resource mch_res;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 00cd17c76fdc..fbd61f92b60d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2891,6 +2891,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 		return;
 	}
 
+	intel_panel_set_orientation_from_crtc(dev_priv->panel, intel_crtc,
+					      plane_config->panel_orientation);
+
 	plane_state->src_x = 0;
 	plane_state->src_y = 0;
 	plane_state->src_w = fb->width << 16;
@@ -7523,6 +7526,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 			fb->modifier = I915_FORMAT_MOD_X_TILED;
 		}
+
+		if (val & DISPPLANE_ROTATE_180)
+			plane_config->panel_orientation +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
 	}
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
@@ -8576,6 +8583,24 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 		goto error;
 	}
 
+	/* The rotation is used to *correct* for the panel orientation */
+	switch (val & PLANE_CTL_ROTATE_MASK) {
+	case PLANE_CTL_ROTATE_0:
+		break;
+	case PLANE_CTL_ROTATE_90:
+		plane_config->panel_orientation +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+		break;
+	case PLANE_CTL_ROTATE_180:
+		plane_config->panel_orientation +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+		break;
+	case PLANE_CTL_ROTATE_270:
+		plane_config->panel_orientation +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+		break;
+	}
+
 	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
 	plane_config->base = base;
 
@@ -8661,6 +8686,10 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 			fb->modifier = I915_FORMAT_MOD_X_TILED;
 		}
+
+		if (val & DISPPLANE_ROTATE_180)
+			plane_config->panel_orientation +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
 	}
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
@@ -14578,7 +14607,9 @@ int intel_modeset_init(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 
 	for_each_intel_crtc(dev, crtc) {
-		struct intel_initial_plane_config plane_config = {};
+		struct intel_initial_plane_config plane_config = {
+			.panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL,
+		};
 
 		if (!crtc->active)
 			continue;
@@ -14600,6 +14631,12 @@ int intel_modeset_init(struct drm_device *dev)
 		intel_find_initial_plane_obj(crtc, &plane_config);
 	}
 
+	/*
+	 * Init panel orientation prop now that intel_find_initial_plane_obj()
+	 * has had a chance to set panel orientation.
+	 */
+	intel_panel_init_orientation_prop(dev_priv->panel);
+
 	/*
 	 * Make sure hardware watermarks really match the state we read out.
 	 * Note that we need to do this after reconstructing the BIOS fb's
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fa47285918f4..50fa0eeca655 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -447,6 +447,7 @@ struct intel_initial_plane_config {
 	unsigned int tiling;
 	int size;
 	u32 base;
+	int panel_orientation; /* DRM_MODE_PANEL_ORIENTATION_* */
 };
 
 #define SKL_MIN_SRC_W 8
@@ -1703,6 +1704,10 @@ extern struct drm_display_mode *intel_find_panel_downclock(
 				struct drm_i915_private *dev_priv,
 				struct drm_display_mode *fixed_mode,
 				struct drm_connector *connector);
+void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
+					   struct intel_crtc *intel_crtc,
+					   int orientation);
+void intel_panel_init_orientation_prop(struct intel_panel *panel);
 
 #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
 int intel_backlight_device_register(struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 0f7942a5dccf..fa7dfb9ac5f0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1925,11 +1925,44 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 	}
 }
 
+void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
+					   struct intel_crtc *intel_crtc,
+					   int orientation)
+{
+	struct intel_connector *panel_conn;
+
+	if (!panel)
+		return;
+
+	panel_conn = container_of(panel, struct intel_connector, panel);
+	if (panel_conn->base.state->crtc = &intel_crtc->base)
+		panel_conn->base.display_info.panel_orientation = orientation;
+}
+
+void intel_panel_init_orientation_prop(struct intel_panel *panel)
+{
+	struct intel_connector *panel_conn;
+
+	if (!panel || !panel->fixed_mode)
+		return;
+
+	panel_conn = container_of(panel, struct intel_connector, panel);
+	drm_connector_init_panel_orientation_property(&panel_conn->base,
+		panel->fixed_mode->hdisplay, panel->fixed_mode->vdisplay);
+}
+
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
 		     struct drm_display_mode *alt_fixed_mode,
 		     struct drm_display_mode *downclock_mode)
 {
+	struct intel_connector *intel_connector +		container_of(panel, struct intel_connector, panel);
+	struct drm_i915_private *dev_priv = to_i915(intel_connector->base.dev);
+
+	if (!dev_priv->panel)
+		dev_priv->panel = panel;
+
 	intel_panel_init_backlight_funcs(panel);
 
 	panel->fixed_mode = fixed_mode;
-- 
2.14.2

Re: [PATCH v2 3/4] drm/i915: Add "panel orientation" property to the panel connector

[Daniel Vetter]

On Sun, Oct 01, 2017 at 05:33:16PM +0200, Hans de Goede wrote:
> Ideally we could use the VBT for this, that would be simple, in
> intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
> connector->display_info.panel_orientation accordingly and call
> drm_connector_init_panel_orientation_property(), done.
> 
> Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
> with an upside down LCD and where the GOP is properly rotating the
> EFI fb in hardware.
> 
> So instead we end up reading the rotation from the primary plane,
> which is a bit more complicated.
> 
> The code to read back the rotation from the hardware is based on an
> earlier attempt to fix fdo#94894 by Ville Syrjala.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This patch looks a bit like it tries to maximize layering violations:

- store panel state in plane_state
- digg a hole from global code to set up panel information

Can't we do this in a better way? I'm thinking of something mimicking the
fixed mode readout, which is also not driven in a backwards way like this.
I.e. a small helper in intel_panel.c which reads out the rotation of the
primary panel and hopes for the best.

Plus ofc taking the quirk list into account.

Also, how exactly does the GOP figure out we need to rotate?
-Daniel

> ---
> Changes in v2:
> -Rebased on 4.14-rc1
> -Readback primary plane rotation from the hardware and use that to
>  set the panel orientation
> -This (readback) fixes fdo#94894, add Fixes: tag
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
>  drivers/gpu/drm/i915/intel_panel.c   | 33 ++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 18d9da53282b..c4c8590972b4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2151,6 +2151,8 @@ struct intel_cdclk_state {
>  	unsigned int cdclk, vco, ref;
>  };
>  
> +struct intel_panel;
> +
>  struct drm_i915_private {
>  	struct drm_device drm;
>  
> @@ -2200,6 +2202,7 @@ struct drm_i915_private {
>  	struct i915_gem_context *kernel_context;
>  	struct intel_engine_cs *engine[I915_NUM_ENGINES];
>  	struct i915_vma *semaphore;
> +	struct intel_panel *panel;
>  
>  	struct drm_dma_handle *status_page_dmah;
>  	struct resource mch_res;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 00cd17c76fdc..fbd61f92b60d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2891,6 +2891,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  		return;
>  	}
>  
> +	intel_panel_set_orientation_from_crtc(dev_priv->panel, intel_crtc,
> +					      plane_config->panel_orientation);
> +
>  	plane_state->src_x = 0;
>  	plane_state->src_y = 0;
>  	plane_state->src_w = fb->width << 16;
> @@ -7523,6 +7526,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>  			plane_config->tiling = I915_TILING_X;
>  			fb->modifier = I915_FORMAT_MOD_X_TILED;
>  		}
> +
> +		if (val & DISPPLANE_ROTATE_180)
> +			plane_config->panel_orientation > +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>  	}
>  
>  	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
> @@ -8576,6 +8583,24 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  		goto error;
>  	}
>  
> +	/* The rotation is used to *correct* for the panel orientation */
> +	switch (val & PLANE_CTL_ROTATE_MASK) {
> +	case PLANE_CTL_ROTATE_0:
> +		break;
> +	case PLANE_CTL_ROTATE_90:
> +		plane_config->panel_orientation > +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> +		break;
> +	case PLANE_CTL_ROTATE_180:
> +		plane_config->panel_orientation > +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> +		break;
> +	case PLANE_CTL_ROTATE_270:
> +		plane_config->panel_orientation > +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> +		break;
> +	}
> +
>  	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
>  	plane_config->base = base;
>  
> @@ -8661,6 +8686,10 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
>  			plane_config->tiling = I915_TILING_X;
>  			fb->modifier = I915_FORMAT_MOD_X_TILED;
>  		}
> +
> +		if (val & DISPPLANE_ROTATE_180)
> +			plane_config->panel_orientation > +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>  	}
>  
>  	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
> @@ -14578,7 +14607,9 @@ int intel_modeset_init(struct drm_device *dev)
>  	drm_modeset_unlock_all(dev);
>  
>  	for_each_intel_crtc(dev, crtc) {
> -		struct intel_initial_plane_config plane_config = {};
> +		struct intel_initial_plane_config plane_config = {
> +			.panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL,
> +		};
>  
>  		if (!crtc->active)
>  			continue;
> @@ -14600,6 +14631,12 @@ int intel_modeset_init(struct drm_device *dev)
>  		intel_find_initial_plane_obj(crtc, &plane_config);
>  	}
>  
> +	/*
> +	 * Init panel orientation prop now that intel_find_initial_plane_obj()
> +	 * has had a chance to set panel orientation.
> +	 */
> +	intel_panel_init_orientation_prop(dev_priv->panel);
> +
>  	/*
>  	 * Make sure hardware watermarks really match the state we read out.
>  	 * Note that we need to do this after reconstructing the BIOS fb's
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index fa47285918f4..50fa0eeca655 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -447,6 +447,7 @@ struct intel_initial_plane_config {
>  	unsigned int tiling;
>  	int size;
>  	u32 base;
> +	int panel_orientation; /* DRM_MODE_PANEL_ORIENTATION_* */
>  };
>  
>  #define SKL_MIN_SRC_W 8
> @@ -1703,6 +1704,10 @@ extern struct drm_display_mode *intel_find_panel_downclock(
>  				struct drm_i915_private *dev_priv,
>  				struct drm_display_mode *fixed_mode,
>  				struct drm_connector *connector);
> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
> +					   struct intel_crtc *intel_crtc,
> +					   int orientation);
> +void intel_panel_init_orientation_prop(struct intel_panel *panel);
>  
>  #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  int intel_backlight_device_register(struct intel_connector *connector);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 0f7942a5dccf..fa7dfb9ac5f0 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1925,11 +1925,44 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>  	}
>  }
>  
> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
> +					   struct intel_crtc *intel_crtc,
> +					   int orientation)
> +{
> +	struct intel_connector *panel_conn;
> +
> +	if (!panel)
> +		return;
> +
> +	panel_conn = container_of(panel, struct intel_connector, panel);
> +	if (panel_conn->base.state->crtc = &intel_crtc->base)
> +		panel_conn->base.display_info.panel_orientation = orientation;
> +}
> +
> +void intel_panel_init_orientation_prop(struct intel_panel *panel)
> +{
> +	struct intel_connector *panel_conn;
> +
> +	if (!panel || !panel->fixed_mode)
> +		return;
> +
> +	panel_conn = container_of(panel, struct intel_connector, panel);
> +	drm_connector_init_panel_orientation_property(&panel_conn->base,
> +		panel->fixed_mode->hdisplay, panel->fixed_mode->vdisplay);
> +}
> +
>  int intel_panel_init(struct intel_panel *panel,
>  		     struct drm_display_mode *fixed_mode,
>  		     struct drm_display_mode *alt_fixed_mode,
>  		     struct drm_display_mode *downclock_mode)
>  {
> +	struct intel_connector *intel_connector > +		container_of(panel, struct intel_connector, panel);
> +	struct drm_i915_private *dev_priv = to_i915(intel_connector->base.dev);
> +
> +	if (!dev_priv->panel)
> +		dev_priv->panel = panel;
> +
>  	intel_panel_init_backlight_funcs(panel);
>  
>  	panel->fixed_mode = fixed_mode;
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 3/4] drm/i915: Add "panel orientation" property to the panel connector

[Hans de Goede]

Hi,

On 02-10-17 10:06, Daniel Vetter wrote:
> On Sun, Oct 01, 2017 at 05:33:16PM +0200, Hans de Goede wrote:
>> Ideally we could use the VBT for this, that would be simple, in
>> intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
>> connector->display_info.panel_orientation accordingly and call
>> drm_connector_init_panel_orientation_property(), done.
>>
>> Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
>> with an upside down LCD and where the GOP is properly rotating the
>> EFI fb in hardware.
>>
>> So instead we end up reading the rotation from the primary plane,
>> which is a bit more complicated.
>>
>> The code to read back the rotation from the hardware is based on an
>> earlier attempt to fix fdo#94894 by Ville Syrjala.
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> This patch looks a bit like it tries to maximize layering violations:
> 
> - store panel state in plane_state
> - digg a hole from global code to set up panel information
> 
> Can't we do this in a better way? I'm thinking of something mimicking the
> fixed mode readout, which is also not driven in a backwards way like this.
> I.e. a small helper in intel_panel.c which reads out the rotation of the
> primary panel and hopes for the best.

Ok, I've reworked this in a way which will hopefully be much more to
your liking. This is now pretty much all isolated to intel_dsi.c

> Plus ofc taking the quirk list into account.
> 
> Also, how exactly does the GOP figure out we need to rotate?

I wish I knew, the logical answer would be the rotation field of
struct mipi_config (which is part of the VBT) but at least no the
tablet with upside-down LCD panel I've using that field is not the
answer, hence the code to read it back from the hw at boot.

Regards,

Hans

>> ---
>> Changes in v2:
>> -Rebased on 4.14-rc1
>> -Readback primary plane rotation from the hardware and use that to
>>   set the panel orientation
>> -This (readback) fixes fdo#94894, add Fixes: tag
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h      |  3 +++
>>   drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/i915/intel_drv.h     |  5 +++++
>>   drivers/gpu/drm/i915/intel_panel.c   | 33 ++++++++++++++++++++++++++++++
>>   4 files changed, 79 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 18d9da53282b..c4c8590972b4 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2151,6 +2151,8 @@ struct intel_cdclk_state {
>>   	unsigned int cdclk, vco, ref;
>>   };
>>   
>> +struct intel_panel;
>> +
>>   struct drm_i915_private {
>>   	struct drm_device drm;
>>   
>> @@ -2200,6 +2202,7 @@ struct drm_i915_private {
>>   	struct i915_gem_context *kernel_context;
>>   	struct intel_engine_cs *engine[I915_NUM_ENGINES];
>>   	struct i915_vma *semaphore;
>> +	struct intel_panel *panel;
>>   
>>   	struct drm_dma_handle *status_page_dmah;
>>   	struct resource mch_res;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 00cd17c76fdc..fbd61f92b60d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2891,6 +2891,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>   		return;
>>   	}
>>   
>> +	intel_panel_set_orientation_from_crtc(dev_priv->panel, intel_crtc,
>> +					      plane_config->panel_orientation);
>> +
>>   	plane_state->src_x = 0;
>>   	plane_state->src_y = 0;
>>   	plane_state->src_w = fb->width << 16;
>> @@ -7523,6 +7526,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>>   			plane_config->tiling = I915_TILING_X;
>>   			fb->modifier = I915_FORMAT_MOD_X_TILED;
>>   		}
>> +
>> +		if (val & DISPPLANE_ROTATE_180)
>> +			plane_config->panel_orientation >> +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>>   	}
>>   
>>   	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>> @@ -8576,6 +8583,24 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>>   		goto error;
>>   	}
>>   
>> +	/* The rotation is used to *correct* for the panel orientation */
>> +	switch (val & PLANE_CTL_ROTATE_MASK) {
>> +	case PLANE_CTL_ROTATE_0:
>> +		break;
>> +	case PLANE_CTL_ROTATE_90:
>> +		plane_config->panel_orientation >> +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
>> +		break;
>> +	case PLANE_CTL_ROTATE_180:
>> +		plane_config->panel_orientation >> +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>> +		break;
>> +	case PLANE_CTL_ROTATE_270:
>> +		plane_config->panel_orientation >> +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
>> +		break;
>> +	}
>> +
>>   	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
>>   	plane_config->base = base;
>>   
>> @@ -8661,6 +8686,10 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
>>   			plane_config->tiling = I915_TILING_X;
>>   			fb->modifier = I915_FORMAT_MOD_X_TILED;
>>   		}
>> +
>> +		if (val & DISPPLANE_ROTATE_180)
>> +			plane_config->panel_orientation >> +				DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
>>   	}
>>   
>>   	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
>> @@ -14578,7 +14607,9 @@ int intel_modeset_init(struct drm_device *dev)
>>   	drm_modeset_unlock_all(dev);
>>   
>>   	for_each_intel_crtc(dev, crtc) {
>> -		struct intel_initial_plane_config plane_config = {};
>> +		struct intel_initial_plane_config plane_config = {
>> +			.panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL,
>> +		};
>>   
>>   		if (!crtc->active)
>>   			continue;
>> @@ -14600,6 +14631,12 @@ int intel_modeset_init(struct drm_device *dev)
>>   		intel_find_initial_plane_obj(crtc, &plane_config);
>>   	}
>>   
>> +	/*
>> +	 * Init panel orientation prop now that intel_find_initial_plane_obj()
>> +	 * has had a chance to set panel orientation.
>> +	 */
>> +	intel_panel_init_orientation_prop(dev_priv->panel);
>> +
>>   	/*
>>   	 * Make sure hardware watermarks really match the state we read out.
>>   	 * Note that we need to do this after reconstructing the BIOS fb's
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index fa47285918f4..50fa0eeca655 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -447,6 +447,7 @@ struct intel_initial_plane_config {
>>   	unsigned int tiling;
>>   	int size;
>>   	u32 base;
>> +	int panel_orientation; /* DRM_MODE_PANEL_ORIENTATION_* */
>>   };
>>   
>>   #define SKL_MIN_SRC_W 8
>> @@ -1703,6 +1704,10 @@ extern struct drm_display_mode *intel_find_panel_downclock(
>>   				struct drm_i915_private *dev_priv,
>>   				struct drm_display_mode *fixed_mode,
>>   				struct drm_connector *connector);
>> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
>> +					   struct intel_crtc *intel_crtc,
>> +					   int orientation);
>> +void intel_panel_init_orientation_prop(struct intel_panel *panel);
>>   
>>   #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   int intel_backlight_device_register(struct intel_connector *connector);
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index 0f7942a5dccf..fa7dfb9ac5f0 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -1925,11 +1925,44 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>>   	}
>>   }
>>   
>> +void intel_panel_set_orientation_from_crtc(struct intel_panel *panel,
>> +					   struct intel_crtc *intel_crtc,
>> +					   int orientation)
>> +{
>> +	struct intel_connector *panel_conn;
>> +
>> +	if (!panel)
>> +		return;
>> +
>> +	panel_conn = container_of(panel, struct intel_connector, panel);
>> +	if (panel_conn->base.state->crtc = &intel_crtc->base)
>> +		panel_conn->base.display_info.panel_orientation = orientation;
>> +}
>> +
>> +void intel_panel_init_orientation_prop(struct intel_panel *panel)
>> +{
>> +	struct intel_connector *panel_conn;
>> +
>> +	if (!panel || !panel->fixed_mode)
>> +		return;
>> +
>> +	panel_conn = container_of(panel, struct intel_connector, panel);
>> +	drm_connector_init_panel_orientation_property(&panel_conn->base,
>> +		panel->fixed_mode->hdisplay, panel->fixed_mode->vdisplay);
>> +}
>> +
>>   int intel_panel_init(struct intel_panel *panel,
>>   		     struct drm_display_mode *fixed_mode,
>>   		     struct drm_display_mode *alt_fixed_mode,
>>   		     struct drm_display_mode *downclock_mode)
>>   {
>> +	struct intel_connector *intel_connector >> +		container_of(panel, struct intel_connector, panel);
>> +	struct drm_i915_private *dev_priv = to_i915(intel_connector->base.dev);
>> +
>> +	if (!dev_priv->panel)
>> +		dev_priv->panel = panel;
>> +
>>   	intel_panel_init_backlight_funcs(panel);
>>   
>>   	panel->fixed_mode = fixed_mode;
>> -- 
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCH v2 4/4] drm/fb-helper: Apply panel orientation connector prop to the primary plane

[Hans de Goede]

Apply the "panel orientation" drm connector prop to the primary plane,
so that fbcon and fbdev using userspace programs display the right way
up.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-New patch in v2 of this patch-set
---
 drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1b8f013ffa65..75c409430a26 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,6 +41,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 
+#include "drm_crtc_internal.h"
 #include "drm_crtc_helper_internal.h"
 
 static bool drm_fbdev_emulation = true;
@@ -347,6 +348,53 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
+static int get_plane_rotation_from_panel_orientation(
+	struct drm_fb_helper *fb_helper, struct drm_plane *plane)
+{
+	int i, rotation = DRM_MODE_ROTATE_0;
+	struct drm_connector *conn;
+	uint64_t valid_mask = 0;
+
+	drm_fb_helper_for_each_connector(fb_helper, i) {
+		conn = fb_helper->connector_info[i]->connector;
+		if (conn->state->crtc && conn->state->crtc->primary = plane) {
+			switch (conn->display_info.panel_orientation) {
+			case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
+				rotation = DRM_MODE_ROTATE_180;
+				break;
+			case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
+				rotation = DRM_MODE_ROTATE_90;
+				break;
+			case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
+				rotation = DRM_MODE_ROTATE_270;
+				break;
+			}
+			break;
+		}
+	}
+
+	/*
+	 * Check the necessary rotation to compensate for the panel orientation
+	 * is supported.
+	 * Note currently we simply leave things as is when not supported, maybe
+	 * we shouls set a hint in fb_info to tell fbcon to rotate in this case
+	 * so that atleast the console ends up the right way. Maybe, but this:
+	 * a) Is not necessary for any known models with a non upright panel
+	 * b) Is tricky because fbcon rotation applies to all outputs rather
+	 *    then a single one
+	 */
+	if (!plane->rotation_property)
+		return DRM_MODE_ROTATE_0;
+
+	for (i = 0; i < plane->rotation_property->num_values; i++)
+		valid_mask |= (1ULL << plane->rotation_property->values[i]);
+
+	if (rotation & ~valid_mask)
+		return DRM_MODE_ROTATE_0;
+
+	return rotation;
+}
+
 static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
 {
 	struct drm_device *dev = fb_helper->dev;
@@ -376,8 +424,9 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
 			goto out_state;
 		}
 
-		plane_state->rotation = DRM_MODE_ROTATE_0;
-
+		plane_state->rotation +			get_plane_rotation_from_panel_orientation(fb_helper,
+								  plane);
 		plane->old_fb = plane->fb;
 		plane_mask |= 1 << drm_plane_index(plane);
 
-- 
2.14.2

Re: [PATCH v2 4/4] drm/fb-helper: Apply panel orientation connector prop to the primary plane

[Daniel Vetter]

On Sun, Oct 01, 2017 at 05:33:17PM +0200, Hans de Goede wrote:
> Apply the "panel orientation" drm connector prop to the primary plane,
> so that fbcon and fbdev using userspace programs display the right way
> up.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -New patch in v2 of this patch-set
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1b8f013ffa65..75c409430a26 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -41,6 +41,7 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  
> +#include "drm_crtc_internal.h"
>  #include "drm_crtc_helper_internal.h"
>  
>  static bool drm_fbdev_emulation = true;
> @@ -347,6 +348,53 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>  
> +static int get_plane_rotation_from_panel_orientation(
> +	struct drm_fb_helper *fb_helper, struct drm_plane *plane)
> +{
> +	int i, rotation = DRM_MODE_ROTATE_0;
> +	struct drm_connector *conn;
> +	uint64_t valid_mask = 0;
> +
> +	drm_fb_helper_for_each_connector(fb_helper, i) {
> +		conn = fb_helper->connector_info[i]->connector;
> +		if (conn->state->crtc && conn->state->crtc->primary = plane) {
> +			switch (conn->display_info.panel_orientation) {
> +			case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> +				rotation = DRM_MODE_ROTATE_180;
> +				break;
> +			case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> +				rotation = DRM_MODE_ROTATE_90;
> +				break;
> +			case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> +				rotation = DRM_MODE_ROTATE_270;
> +				break;
> +			}
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * Check the necessary rotation to compensate for the panel orientation
> +	 * is supported.
> +	 * Note currently we simply leave things as is when not supported, maybe
> +	 * we shouls set a hint in fb_info to tell fbcon to rotate in this case
> +	 * so that atleast the console ends up the right way. Maybe, but this:
> +	 * a) Is not necessary for any known models with a non upright panel
> +	 * b) Is tricky because fbcon rotation applies to all outputs rather
> +	 *    then a single one
> +	 */
> +	if (!plane->rotation_property)
> +		return DRM_MODE_ROTATE_0;
> +
> +	for (i = 0; i < plane->rotation_property->num_values; i++)
> +		valid_mask |= (1ULL << plane->rotation_property->values[i]);
> +
> +	if (rotation & ~valid_mask)
> +		return DRM_MODE_ROTATE_0;
> +
> +	return rotation;
> +}
> +
>  static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
>  {
>  	struct drm_device *dev = fb_helper->dev;
> @@ -376,8 +424,9 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
>  			goto out_state;
>  		}
>  
> -		plane_state->rotation = DRM_MODE_ROTATE_0;
> -
> +		plane_state->rotation > +			get_plane_rotation_from_panel_orientation(fb_helper,
> +								  plane);

This is the loop to reset all the planes to default values, feels a bit
awkward to change the rotation value in there. Also gives you that
unpretty loop to check you're on the right plane.

I think a cleaner way to do this would be:
- add a rotation member to struct drm_fb_helper_crtc
- set that when setting up the configuration drm_setup_crtcs
- look up crtc->primary plane_state and set it int the loop below this one
  here (which is the one that actually sets the mode)

Cheers, Daniel
>  		plane->old_fb = plane->fb;
>  		plane_mask |= 1 << drm_plane_index(plane);
>  
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 4/4] drm/fb-helper: Apply panel orientation connector prop to the primary plane

[Hans de Goede]

Hi,

On 02-10-17 10:15, Daniel Vetter wrote:
> On Sun, Oct 01, 2017 at 05:33:17PM +0200, Hans de Goede wrote:
>> Apply the "panel orientation" drm connector prop to the primary plane,
>> so that fbcon and fbdev using userspace programs display the right way
>> up.
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -New patch in v2 of this patch-set
>> ---
>>   drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 51 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 1b8f013ffa65..75c409430a26 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -41,6 +41,7 @@
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_atomic_helper.h>
>>   
>> +#include "drm_crtc_internal.h"
>>   #include "drm_crtc_helper_internal.h"
>>   
>>   static bool drm_fbdev_emulation = true;
>> @@ -347,6 +348,53 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>>   }
>>   EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>>   
>> +static int get_plane_rotation_from_panel_orientation(
>> +	struct drm_fb_helper *fb_helper, struct drm_plane *plane)
>> +{
>> +	int i, rotation = DRM_MODE_ROTATE_0;
>> +	struct drm_connector *conn;
>> +	uint64_t valid_mask = 0;
>> +
>> +	drm_fb_helper_for_each_connector(fb_helper, i) {
>> +		conn = fb_helper->connector_info[i]->connector;
>> +		if (conn->state->crtc && conn->state->crtc->primary = plane) {
>> +			switch (conn->display_info.panel_orientation) {
>> +			case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
>> +				rotation = DRM_MODE_ROTATE_180;
>> +				break;
>> +			case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
>> +				rotation = DRM_MODE_ROTATE_90;
>> +				break;
>> +			case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
>> +				rotation = DRM_MODE_ROTATE_270;
>> +				break;
>> +			}
>> +			break;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * Check the necessary rotation to compensate for the panel orientation
>> +	 * is supported.
>> +	 * Note currently we simply leave things as is when not supported, maybe
>> +	 * we shouls set a hint in fb_info to tell fbcon to rotate in this case
>> +	 * so that atleast the console ends up the right way. Maybe, but this:
>> +	 * a) Is not necessary for any known models with a non upright panel
>> +	 * b) Is tricky because fbcon rotation applies to all outputs rather
>> +	 *    then a single one
>> +	 */
>> +	if (!plane->rotation_property)
>> +		return DRM_MODE_ROTATE_0;
>> +
>> +	for (i = 0; i < plane->rotation_property->num_values; i++)
>> +		valid_mask |= (1ULL << plane->rotation_property->values[i]);
>> +
>> +	if (rotation & ~valid_mask)
>> +		return DRM_MODE_ROTATE_0;
>> +
>> +	return rotation;
>> +}
>> +
>>   static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
>>   {
>>   	struct drm_device *dev = fb_helper->dev;
>> @@ -376,8 +424,9 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
>>   			goto out_state;
>>   		}
>>   
>> -		plane_state->rotation = DRM_MODE_ROTATE_0;
>> -
>> +		plane_state->rotation >> +			get_plane_rotation_from_panel_orientation(fb_helper,
>> +								  plane);
> 
> This is the loop to reset all the planes to default values, feels a bit
> awkward to change the rotation value in there. Also gives you that
> unpretty loop to check you're on the right plane.
> 
> I think a cleaner way to do this would be:
> - add a rotation member to struct drm_fb_helper_crtc
> - set that when setting up the configuration drm_setup_crtcs
> - look up crtc->primary plane_state and set it int the loop below this one
>    here (which is the one that actually sets the mode)

Ah yes much better, I've done this for v3. Thank you for the hints.

Regards,

Hans



> 
> Cheers, Daniel
>>   		plane->old_fb = plane->fb;
>>   		plane_mask |= 1 << drm_plane_index(plane);
>>   
>> -- 
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCH v2 1/4] video: fb: Make fbcon dmi quirks usuable for drm drivers too

[Hans de Goede]

Some x86 clamshell design devices use portrait tablet LCD panels and a
display engine which cannot (transparently) rotate in hardware, so we
need to rotate things in software / let user space deal with this.

The fbcon code has a set of DMI based quirks to automatically detect
such tablet LCD panels and rotate the fbcon to compensate.

The plan was for userspace (e.g. a Wayland compositor) to simply read
/sys/class/graphics/fbcon/rotate and apply the rotation from there to
the LCD panel to compensate.

However this will not work when an external monitor gets plugged in,
since with fbcon rotation is not per output, so the fbcon quirk code
disables the rotation when an external monitor is present.

Using /sys/class/graphics/fbcon/rotate will not help in that case
since it will indicate no rotation is in use. So instead we are going
to need a drm connecter property for this.

This commit is a preparation patch for adding the drm-connecter
property, by making the fbcon quirk code generally usable so that
the drm code can use it to check for rotation quirks.

Note this commit re-uses the FB_CMDLINE Kconfig option for selecting
if the quirk code should be build, since that is already selected by
both the drm and fbcon code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rebased on 4.14-rc1
---
 drivers/video/fbdev/core/Makefile                  |  6 +++---
 .../core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c}   | 15 +++++++++------
 drivers/video/fbdev/core/fbcon.c                   | 22 ++++++++++++++--------
 drivers/video/fbdev/core/fbcon.h                   |  6 ------
 include/linux/fb.h                                 |  6 ++++++
 5 files changed, 32 insertions(+), 23 deletions(-)
 rename drivers/video/fbdev/core/{fbcon_dmi_quirks.c => fb_dmi_quirks.c} (91%)

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 73493bbd7a15..06caf037a822 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,7 @@
 obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
+ifeq ($(CONFIG_DMI),y)
+obj-$(CONFIG_FB_CMDLINE)          += fb_dmi_quirks.o
+endif
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
@@ -14,9 +17,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
 fb-y				  += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
 				     fbcon_ccw.o
 endif
-ifeq ($(CONFIG_DMI),y)
-fb-y				  += fbcon_dmi_quirks.o
-endif
 endif
 fb-objs                           := $(fb-y)
 
diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c b/drivers/video/fbdev/core/fb_dmi_quirks.c
similarity index 91%
rename from drivers/video/fbdev/core/fbcon_dmi_quirks.c
rename to drivers/video/fbdev/core/fb_dmi_quirks.c
index 6904e47d1e51..d5fdf3245f83 100644
--- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
+++ b/drivers/video/fbdev/core/fb_dmi_quirks.c
@@ -1,5 +1,5 @@
 /*
- *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
+ *  fb_dmi_quirks.c -- DMI based LCD panel rotation quirk detection
  *
  *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
  *
@@ -11,7 +11,6 @@
 #include <linux/dmi.h>
 #include <linux/fb.h>
 #include <linux/kernel.h>
-#include "fbcon.h"
 
 /*
  * Some x86 clamshell design devices use portrait tablet screens and a display
@@ -112,7 +111,11 @@ static const struct dmi_system_id rotate_data[] = {
 	{}
 };
 
-int fbcon_platform_get_rotate(struct fb_info *info)
+/*
+ * Note this function returns the rotation necessary to put the display
+ * the right way up, or -1 if there is no quirk.
+ */
+int fb_get_panel_rotate_quirk(int width, int height)
 {
 	const struct dmi_system_id *match;
 	const struct fbcon_dmi_rotate_data *data;
@@ -124,8 +127,7 @@ int fbcon_platform_get_rotate(struct fb_info *info)
 	     match = dmi_first_match(match + 1)) {
 		data = match->driver_data;
 
-		if (data->width != info->var.xres ||
-		    data->height != info->var.yres)
+		if (data->width != width || data->height != height)
 			continue;
 
 		if (!data->bios_dates)
@@ -141,5 +143,6 @@ int fbcon_platform_get_rotate(struct fb_info *info)
 		}
 	}
 
-	return FB_ROTATE_UR;
+	return -1;
 }
+EXPORT_SYMBOL_GPL(fb_get_panel_rotate_quirk);
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 04612f938bab..2e17ea02c295 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
 	ops->cur_rotate = -1;
 	ops->cur_blink_jiffies = HZ / 5;
 	info->fbcon_par = ops;
-	if (initial_rotation != -1)
-		p->con_rotate = initial_rotation;
-	else
-		p->con_rotate = fbcon_platform_get_rotate(info);
+	p->con_rotate = initial_rotation;
+	if (p->con_rotate = -1)
+		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
+							  info->var.yres);
+	if (p->con_rotate = -1)
+		p->con_rotate = FB_ROTATE_UR;
+
 	set_blitting_type(vc, info);
 
 	if (info->fix.type != FB_TYPE_TEXT) {
@@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
 
 	ops = info->fbcon_par;
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	if (initial_rotation != -1)
-		p->con_rotate = initial_rotation;
-	else
-		p->con_rotate = fbcon_platform_get_rotate(info);
+	p->con_rotate = initial_rotation;
+	if (p->con_rotate = -1)
+		p->con_rotate = fb_get_panel_rotate_quirk(info->var.xres,
+							  info->var.yres);
+	if (p->con_rotate = -1)
+		p->con_rotate = FB_ROTATE_UR;
+
 	set_blitting_type(vc, info);
 
 	cols = vc->vc_cols;
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 18f3ac144237..3746828356ed 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
 #define fbcon_set_rotate(x) do {} while(0)
 #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
 
-#ifdef CONFIG_DMI
-int fbcon_platform_get_rotate(struct fb_info *info);
-#else
-#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
-#endif /* CONFIG_DMI */
-
 #endif /* _VIDEO_FBCON_H */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f4386b0ccf40..7527965c5b53 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -814,6 +814,12 @@ extern int fb_find_mode(struct fb_var_screeninfo *var,
 			const struct fb_videomode *default_mode,
 			unsigned int default_bpp);
 
+#ifdef CONFIG_DMI
+int fb_get_panel_rotate_quirk(int width, int height);
+#else
+#define fb_get_panel_rotate_quirk(width, height) (-1)
+#endif /* CONFIG_DMI */
+
 /* Convenience logging macros */
 #define fb_err(fb_info, fmt, ...)					\
 	pr_err("fb%d: " fmt, (fb_info)->node, ##__VA_ARGS__)
-- 
2.14.1

[PATCH v2 4/4] drm/fb-helper: Apply panel orientation connector prop to the primary plane

[Hans de Goede]

Apply the "panel orientation" drm connector prop to the primary plane,
so that fbcon and fbdev using userspace programs display the right way
up.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id”894
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-New patch in v2 of this patch-set
---
 drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1b8f013ffa65..75c409430a26 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,6 +41,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 
+#include "drm_crtc_internal.h"
 #include "drm_crtc_helper_internal.h"
 
 static bool drm_fbdev_emulation = true;
@@ -347,6 +348,53 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
+static int get_plane_rotation_from_panel_orientation(
+	struct drm_fb_helper *fb_helper, struct drm_plane *plane)
+{
+	int i, rotation = DRM_MODE_ROTATE_0;
+	struct drm_connector *conn;
+	uint64_t valid_mask = 0;
+
+	drm_fb_helper_for_each_connector(fb_helper, i) {
+		conn = fb_helper->connector_info[i]->connector;
+		if (conn->state->crtc && conn->state->crtc->primary = plane) {
+			switch (conn->display_info.panel_orientation) {
+			case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
+				rotation = DRM_MODE_ROTATE_180;
+				break;
+			case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
+				rotation = DRM_MODE_ROTATE_90;
+				break;
+			case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
+				rotation = DRM_MODE_ROTATE_270;
+				break;
+			}
+			break;
+		}
+	}
+
+	/*
+	 * Check the necessary rotation to compensate for the panel orientation
+	 * is supported.
+	 * Note currently we simply leave things as is when not supported, maybe
+	 * we shouls set a hint in fb_info to tell fbcon to rotate in this case
+	 * so that atleast the console ends up the right way. Maybe, but this:
+	 * a) Is not necessary for any known models with a non upright panel
+	 * b) Is tricky because fbcon rotation applies to all outputs rather
+	 *    then a single one
+	 */
+	if (!plane->rotation_property)
+		return DRM_MODE_ROTATE_0;
+
+	for (i = 0; i < plane->rotation_property->num_values; i++)
+		valid_mask |= (1ULL << plane->rotation_property->values[i]);
+
+	if (rotation & ~valid_mask)
+		return DRM_MODE_ROTATE_0;
+
+	return rotation;
+}
+
 static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
 {
 	struct drm_device *dev = fb_helper->dev;
@@ -376,8 +424,9 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
 			goto out_state;
 		}
 
-		plane_state->rotation = DRM_MODE_ROTATE_0;
-
+		plane_state->rotation +			get_plane_rotation_from_panel_orientation(fb_helper,
+								  plane);
 		plane->old_fb = plane->fb;
 		plane_mask |= 1 << drm_plane_index(plane);
 
-- 
2.14.1