linux-kernel - Re: [v8,2/4] drm/panel: set display info in panel attach

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 29 Sep 2019 05:23:14 +0000
From:   "james qian wang (Arm Technology China)" <james.qian.wang@....com>
To:     Derek Basehore <dbasehore@...omium.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Sam Ravnborg <sam@...nborg.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        David Airlie <airlied@...ux.ie>,
        Thierry Reding <thierry.reding@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>, Sean Paul <sean@...rly.run>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>, nd <nd@....com>
Subject: Re: [v8,2/4] drm/panel: set display info in panel attach

On Wed, Sep 25, 2019 at 03:58:31PM -0700, Derek Basehore wrote:
> Devicetree systems can set panel orientation via a panel binding, but
> there's no way, as is, to propagate this setting to the connector,
> where the property need to be added.
> To address this, this patch sets orientation, as well as other fixed
> values for the panel, in the drm_panel_attach function. These values
> are stored from probe in the drm_panel struct.
> 
> Signed-off-by: Derek Basehore <dbasehore@...omium.org>
> Reviewed-by: Sam Ravnborg <sam@...nborg.org>
> ---
>  drivers/gpu/drm/drm_panel.c | 28 +++++++++++++++++++++
>  include/drm/drm_panel.h     | 50 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 0909b53b74e6..1cd2b56c9fe6 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -104,11 +104,23 @@ EXPORT_SYMBOL(drm_panel_remove);
>   */
>  int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector)
>  {
> +	struct drm_display_info *info;
> +
>  	if (panel->connector)
>  		return -EBUSY;
>  
>  	panel->connector = connector;
>  	panel->drm = connector->dev;
> +	info = &connector->display_info;
> +	info->width_mm = panel->width_mm;
> +	info->height_mm = panel->height_mm;
> +	info->bpc = panel->bpc;
> +	info->panel_orientation = panel->orientation;
> +	info->bus_flags = panel->bus_flags;
> +	if (panel->bus_formats)
> +		drm_display_info_set_bus_formats(&connector->display_info,
> +						 panel->bus_formats,
> +						 panel->num_bus_formats);
>  
>  	return 0;
>  }
> @@ -126,6 +138,22 @@ EXPORT_SYMBOL(drm_panel_attach);
>   */
>  void drm_panel_detach(struct drm_panel *panel)
>  {
> +	struct drm_display_info *info;
> +
> +	if (!panel->connector)
> +		goto out;
> +
> +	info = &panel->connector->display_info;
> +	info->width_mm = 0;
> +	info->height_mm = 0;
> +	info->bpc = 0;
> +	info->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +	info->bus_flags = 0;
> +	kfree(info->bus_formats);
> +	info->bus_formats = NULL;
> +	info->num_bus_formats = 0;
> +
> +out:
>  	panel->connector = NULL;
>  	panel->drm = NULL;
>  }
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index d16158deacdc..f3587a54b8ac 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -141,6 +141,56 @@ struct drm_panel {
>  	 */
>  	const struct drm_panel_funcs *funcs;
>

All these new added members seems dupliated with drm_display_info,
So I think, can we add a new drm_plane_funcs func:

int (*set_display_info)(struct drm_panel *panel,
                        struct drm_display_info *info);

Then in drm_panel_attach(), via this interface the specific panel
driver can directly set connector->display_info. like

   ...
   if (panel->funcs && panel->funcs->set_display_info)
		panel->funcs->unprepare(panel, connector->display_info);
   ...

Thanks
James

> +	/**
> +	 * @width_mm:
> +	 *
> +	 * Physical width in mm.
> +	 */
> +	unsigned int width_mm;
> +
> +	/**
> +	 * @height_mm:
> +	 *
> +	 * Physical height in mm.
> +	 */
> +	unsigned int height_mm;
> +
> +	/**
> +	 * @bpc:
> +	 *
> +	 * Maximum bits per color channel. Used by HDMI and DP outputs.
> +	 */
> +	unsigned int bpc;
> +
> +	/**
> +	 * @orientation
> +	 *
> +	 * Installation orientation of the panel with respect to the chassis.
> +	 */
> +	int orientation;
> +
> +	/**
> +	 * @bus_formats
> +	 *
> +	 * Pixel data format on the wire.
> +	 */
> +	const u32 *bus_formats;
> +
> +	/**
> +	 * @num_bus_formats:
> +	 *
> +	 * Number of elements pointed to by @bus_formats
> +	 */
> +	unsigned int num_bus_formats;
> +
> +	/**
> +	 * @bus_flags:
> +	 *
> +	 * Additional information (like pixel signal polarity) for the pixel
> +	 * data on the bus.
> +	 */
> +	u32 bus_flags;
> +
>  	/**
>  	 * @list:
>  	 *