linux-kernel - Re: [RFC PATCH] drm/bridge: ti-sn65dsi86: Enable HPD functionality

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]

Message-ID: <3f44ec0b-216c-4534-a6de-7b17929cb9e1@ti.com>
Date: Fri, 25 Apr 2025 07:02:18 +0530
From: "Kumar, Udit" <u-kumar1@...com>
To: Jayesh Choudhary <j-choudhary@...com>, <dianders@...omium.org>,
        <andrzej.hajda@...el.com>, <neil.armstrong@...aro.org>,
        <rfoss@...nel.org>, <Laurent.pinchart@...asonboard.com>,
        <dri-devel@...ts.freedesktop.org>, <tomi.valkeinen@...asonboard.com>
CC: <jonas@...boo.se>, <jernej.skrabec@...il.com>,
        <maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
        <tzimmermann@...e.de>, <airlied@...il.com>, <simona@...ll.ch>,
        <linux-kernel@...r.kernel.org>, <u-kumar1@...com>
Subject: Re: [RFC PATCH] drm/bridge: ti-sn65dsi86: Enable HPD functionality

Hello Jayesh,

On 4/24/2025 4:24 PM, Jayesh Choudhary wrote:
> For TI SoC J784S4, the display pipeline looks like:
> TIDSS -> CDNS-DSI -> SN65DSI86 -> DisplayConnector -> DisplaySink
> This requires HPD to detect connection form the connector.
> By default, the HPD is disabled for eDP. So enable it conditionally
> based on a new flag 'keep-hpd' as mentioned in the comments in the
> driver.
>
> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
> ---
>
> Hello All,
>
> Sending this RFC patch to get some thoughts on hpd for sn65dsi86.
>
> Now that we have a usecase for hpd in sn65dsi86, I wanted to get
> some comments on this approach to "NOT DISABLE" hpd in the bridge.
> As the driver considers the eDP case, it disables hpd by default.
> So I have added another property in the binding for keeping hpd
> functionality (the name used is still debatable) and used it in
> the driver.
>
> Is this approach okay?
> Also should this have a "Fixes" tag?

>
>   .../bindings/display/bridge/ti,sn65dsi86.yaml      |  6 ++++++
>   drivers/gpu/drm/bridge/ti-sn65dsi86.c              | 14 +++++++++-----
>   2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> index c93878b6d718..5948be612849 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml
> @@ -34,6 +34,12 @@ properties:
>         Set if the HPD line on the bridge isn't hooked up to anything or is
>         otherwise unusable.
>   
> +  keep-hpd:
> +    type: boolean
> +    description:
> +      HPD is disabled in the bridge by default. Set it if HPD line makes
> +      sense and is used.
> +

Here are my suggestions

1) use interrupt in binding as optional instead of keep-hpd

2) use interrupt field (if present to enable of disable HPD functions in 
driver)


>     vccio-supply:
>       description: A 1.8V supply that powers the digital IOs.
>   
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f72675766e01..4081cc957c6c 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -191,6 +191,7 @@ struct ti_sn65dsi86 {
>   	u8				ln_polrs;
>   	bool				comms_enabled;
>   	struct mutex			comms_mutex;
> +	bool				keep_hpd;
>   
>   #if defined(CONFIG_OF_GPIO)
>   	struct gpio_chip		gchip;
> @@ -348,12 +349,15 @@ static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata,
>   	 * 200 ms.  We'll assume that the panel driver will have the hardcoded
>   	 * delay in its prepare and always disable HPD.
>   	 *
> -	 * If HPD somehow makes sense on some future panel we'll have to
> -	 * change this to be conditional on someone specifying that HPD should
> -	 * be used.
> +	 * If needed, use 'keep-hpd' property in the hardware description in
> +	 * board file as a conditional specifying that HPD should be used.
>   	 */
> -	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
> -			   HPD_DISABLE);
> +
> +	pdata->keep_hpd = of_property_read_bool(pdata->dev->of_node, "keep-hpd");
> +
> +	if (!pdata->keep_hpd)
> +		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
> +				   HPD_DISABLE);

you may want to extend interrupt support in this driver if HPD is enabled.


>   
>   	pdata->comms_enabled = true;
>