Re: [PATCH] video/console: Add dmi quirk table for x86 systems which need fbcon rotation

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

Hi,

On Tuesday, June 27, 2017 11:13:21 PM Hans de Goede wrote:
> Some x86 clamshell design devices use portrait tablet screens and a
> display engine which cannot rotate in hardware, so we need to rotate
> the fbcon to compensate.
> 
> This commit adds a DMI based quirk table which is initially populated with
> 3 such devices: The GPD Win, the GPD Pocket and the I.T.Works TW891, so
> that the console comes up in the right orientation on this devices OOTB.
> 
> Unfortunately these (cheap) devices also typically have quite generic DMI
> data, so we match on a combination of DMI data, screen resolution and a
> list of known BIOS dates to avoid false positives.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/firmware/dmi_scan.c              |   3 +-
>  drivers/video/console/Makefile           |   3 +
>  drivers/video/console/fbcon.c            |  12 +++-
>  drivers/video/console/fbcon.h            |   7 ++-
>  drivers/video/console/fbcon_dmi_quirks.c | 103 +++++++++++++++++++++++++++++++
>  include/linux/dmi.h                      |   1 +
>  6 files changed, 124 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/video/console/fbcon_dmi_quirks.c
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 7830419..bb1ad8b 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -780,7 +780,7 @@ void __init dmi_set_dump_stack_arch_desc(void)
>   *	dmi_matches - check if dmi_system_id structure matches system DMI data
>   *	@dmi: pointer to the dmi_system_id structure to check
>   */
> -static bool dmi_matches(const struct dmi_system_id *dmi)
> +bool dmi_matches(const struct dmi_system_id *dmi)
>  {
>  	int i;
>  
> @@ -804,6 +804,7 @@ static bool dmi_matches(const struct dmi_system_id *dmi)
>  	}
>  	return true;
>  }
> +EXPORT_SYMBOL(dmi_matches);

Please explain why dmi_check_system() cannot be used.

+ This needs an Ack from Jean (added to Cc:).

The rest looks fine to me.

>  /**
>   *	dmi_is_end_of_table - check for end-of-table marker
> diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile
> index 43bfa48..32ee2ad 100644
> --- a/drivers/video/console/Makefile
> +++ b/drivers/video/console/Makefile
> @@ -15,5 +15,8 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
>  obj-$(CONFIG_FRAMEBUFFER_CONSOLE)     += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
>                                           fbcon_ccw.o
>  endif
> +ifeq ($(CONFIG_DMI),y)
> +obj-$(CONFIG_FRAMEBUFFER_CONSOLE)     += fbcon_dmi_quirks.o
> +endif
>  
>  obj-$(CONFIG_FB_STI)              += sticore.o
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 12ded23..3db5ac2 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -135,7 +135,7 @@ static char fontname[40];
>  static int info_idx = -1;
>  
>  /* console rotation */
> -static int initial_rotation;
> +static int initial_rotation = -1;
>  static int fbcon_has_sysfs;
>  
>  static const struct consw fb_con;
> @@ -954,7 +954,10 @@ static const char *fbcon_startup(void)
>  	ops->cur_rotate = -1;
>  	ops->cur_blink_jiffies = HZ / 5;
>  	info->fbcon_par = ops;
> -	p->con_rotate = initial_rotation;
> +	if (initial_rotation != -1)
> +		p->con_rotate = initial_rotation;
> +	else
> +		p->con_rotate = fbcon_platform_get_rotate(info);
>  	set_blitting_type(vc, info);
>  
>  	if (info->fix.type != FB_TYPE_TEXT) {
> @@ -1091,7 +1094,10 @@ 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);
> -	p->con_rotate = initial_rotation;
> +	if (initial_rotation != -1)
> +		p->con_rotate = initial_rotation;
> +	else
> +		p->con_rotate = fbcon_platform_get_rotate(info);
>  	set_blitting_type(vc, info);
>  
>  	cols = vc->vc_cols;
> diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
> index 7aaa4ea..60e25e1 100644
> --- a/drivers/video/console/fbcon.h
> +++ b/drivers/video/console/fbcon.h
> @@ -261,5 +261,10 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
>  #define fbcon_set_rotate(x) do {} while(0)
>  #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
>  
> -#endif /* _VIDEO_FBCON_H */
> +#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/drivers/video/console/fbcon_dmi_quirks.c b/drivers/video/console/fbcon_dmi_quirks.c
> new file mode 100644
> index 0000000..3267cab
> --- /dev/null
> +++ b/drivers/video/console/fbcon_dmi_quirks.c
> @@ -0,0 +1,103 @@
> +/*
> + *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
> + *
> + *	Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> + *
> + *  This file is subject to the terms and conditions of the GNU General Public
> + *  License.  See the file COPYING in the main directory of this archive for
> + *  more details.
> + */
> +
> +#include <linux/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
> + * engine which cannot rotate in hardware, so we need to rotate the fbcon to
> + * compensate. Unfortunately these (cheap) devices also typically have quite
> + * generic DMI data, so we match on a combination of DMI data, screen resolution
> + * and a list of known BIOS dates to avoid false positives.
> + */
> +
> +struct fbcon_dmi_rotate_data {
> +	struct dmi_system_id dmi_id;
> +	int width;
> +	int height;
> +	const char * const *bios_dates;
> +	int rotate;
> +};
> +
> +static const struct fbcon_dmi_rotate_data rotate_data[] = {
> +	{	/*
> +		 * GPD Win, note that the the DMI data is less generic then it
> +		 * seems, devices with a board_vendor of "AMI Corporation" are
> +		 * quite rare, as are devices which have both board- *and*
> +		 * product-id set to "Default String"
> +		 */
> +		.dmi_id = { .matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
> +			DMI_MATCH(DMI_BOARD_NAME, "Default string"),
> +			DMI_MATCH(DMI_BOARD_SERIAL, "Default string"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
> +		} },
> +		.width = 720,
> +		.height = 1280,
> +		.bios_dates = (const char * const []){
> +			"10/25/2016", "11/18/2016", "02/21/2017",
> +			"03/20/2017", NULL },
> +		.rotate = FB_ROTATE_CW
> +	}, {	/* GPD Pocket (same note on DMI match as GPD Win) */
> +		.dmi_id = { .matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
> +			DMI_MATCH(DMI_BOARD_NAME, "Default string"),
> +			DMI_MATCH(DMI_BOARD_SERIAL, "Default string"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
> +		} },
> +		.width = 1200,
> +		.height = 1920,
> +		.bios_dates = (const char * const []){ "05/26/2017", NULL },
> +		.rotate = FB_ROTATE_CW,
> +	}, {	/* I.T.Works TW891 */
> +		.dmi_id = { .matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "To be filled by O.E.M."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "TW891"),
> +			DMI_MATCH(DMI_BOARD_VENDOR, "To be filled by O.E.M."),
> +			DMI_MATCH(DMI_BOARD_NAME, "TW891"),
> +		} },
> +		.width = 800,
> +		.height = 1280,
> +		.bios_dates = (const char * const []){ "10/16/2015", NULL },
> +		.rotate = FB_ROTATE_CW,
> +	}
> +};
> +
> +int fbcon_platform_get_rotate(struct fb_info *info)
> +{
> +	const char *bios_date;
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(rotate_data); i++) {
> +		if (!dmi_matches(&rotate_data[i].dmi_id))
> +			continue;
> +
> +		if (rotate_data[i].width != info->var.xres ||
> +		    rotate_data[i].height != info->var.yres)
> +			continue;
> +
> +		if (!rotate_data[i].bios_dates)
> +			return rotate_data->rotate;
> +
> +		bios_date = dmi_get_system_info(DMI_BIOS_DATE);
> +		if (!bios_date)
> +			continue;
> +
> +		for (j = 0; rotate_data[i].bios_dates[j]; j++) {
> +			if (!strcmp(rotate_data[i].bios_dates[j], bios_date))
> +				return rotate_data->rotate;
> +		}
> +	}
> +
> +	return FB_ROTATE_UR;
> +}
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 9bbf21a..f1d28af 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -98,6 +98,7 @@ struct dmi_dev_onboard {
>  extern struct kobject *dmi_kobj;
>  extern int dmi_check_system(const struct dmi_system_id *list);
>  const struct dmi_system_id *dmi_first_match(const struct dmi_system_id *list);
> +bool dmi_matches(const struct dmi_system_id *dmi);
>  extern const char * dmi_get_system_info(int field);
>  extern const struct dmi_device * dmi_find_device(int type, const char *name,
>  	const struct dmi_device *from);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics