All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Jonas Karlman <jonas@kwiboo.se>
Cc: Archit Taneja <architt@codeaurora.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	David Airlie <airlied@linux.ie>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Sean Paul <seanpaul@chromium.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH] drm/bridge: dw-hdmi: fix SCDC configuration for ddc-i2c-bus
Date: Tue, 23 Apr 2019 12:26:02 +0200	[thread overview]
Message-ID: <2275367.P1g2IIHPGP@phil> (raw)
In-Reply-To: <VE1PR03MB59031814B5BCAB2152923BDAAC210@VE1PR03MB5903.eurprd03.prod.outlook.com>

Am Sonntag, 21. April 2019, 10:25:50 CEST schrieb Jonas Karlman:
> When ddc-i2c-bus property is used, a NULL pointer dereference is reported:
> 
> [   31.041669] Unable to handle kernel NULL pointer dereference at virtual address 00000008
> [   31.041671] pgd = 4d3c16f6
> [   31.041673] [00000008] *pgd=00000000
> [   31.041678] Internal error: Oops: 5 [#1] SMP ARM
> 
> [   31.041711] Hardware name: Rockchip (Device Tree)
> [   31.041718] PC is at i2c_transfer+0x8/0xe4
> [   31.041721] LR is at drm_scdc_read+0x54/0x84
> [   31.041723] pc : [<c073273c>]    lr : [<c05926c4>]    psr: 280f0013
> [   31.041725] sp : edffdad0  ip : 5ccb5511  fp : 00000058
> [   31.041727] r10: 00000780  r9 : edf91608  r8 : c11b0f48
> [   31.041728] r7 : 00000438  r6 : 00000000  r5 : 00000000  r4 : 00000000
> [   31.041730] r3 : edffdae7  r2 : 00000002  r1 : edffdaec  r0 : 00000000
> 
> [   31.041908] [<c073273c>] (i2c_transfer) from [<c05926c4>] (drm_scdc_read+0x54/0x84)
> [   31.041913] [<c05926c4>] (drm_scdc_read) from [<c0592858>] (drm_scdc_set_scrambling+0x30/0xbc)
> [   31.041919] [<c0592858>] (drm_scdc_set_scrambling) from [<c05cc0f4>] (dw_hdmi_update_power+0x1440/0x1610)
> [   31.041926] [<c05cc0f4>] (dw_hdmi_update_power) from [<c05cc574>] (dw_hdmi_bridge_enable+0x2c/0x70)
> [   31.041932] [<c05cc574>] (dw_hdmi_bridge_enable) from [<c05aed48>] (drm_bridge_enable+0x24/0x34)
> [   31.041938] [<c05aed48>] (drm_bridge_enable) from [<c0591060>] (drm_atomic_helper_commit_modeset_enables+0x114/0x220)
> [   31.041943] [<c0591060>] (drm_atomic_helper_commit_modeset_enables) from [<c05c3fe0>] (rockchip_atomic_helper_commit_tail_rpm+0x28/0x64)
> 
> hdmi->i2c may not be set when ddc-i2c-bus property is used in device tree.
> Fix this by using hdmi->ddc as the i2c adapter when calling drm_scdc_*().
> Also report that SCDC is not supported when there is no DDC bus.
> 
> Fixes: 264fce6cc2c1 ("drm/bridge: dw-hdmi: Add SCDC and TMDS Scrambling support")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

I've looked that up and yeah, hdmi->ddc seems the right thing to use
as it points either to the external i2c or the internal one, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 7a47c65d8ecb..fd1c319a4ee0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1046,6 +1046,10 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi)
>  	if (hdmi->version < 0x200a)
>  		return false;
>  
> +	/* Disable if no DDC bus */
> +	if (!hdmi->ddc)
> +		return false;
> +
>  	/* Disable if SCDC is not supported, or if an HF-VSDB block is absent */
>  	if (!display->hdmi.scdc.supported ||
>  	    !display->hdmi.scdc.scrambling.supported)
> @@ -1683,13 +1687,13 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			 * Source Devices compliant shall set the
>  			 * Source Version = 1.
>  			 */
> -			drm_scdc_readb(&hdmi->i2c->adap, SCDC_SINK_VERSION,
> +			drm_scdc_readb(hdmi->ddc, SCDC_SINK_VERSION,
>  				       &bytes);
> -			drm_scdc_writeb(&hdmi->i2c->adap, SCDC_SOURCE_VERSION,
> +			drm_scdc_writeb(hdmi->ddc, SCDC_SOURCE_VERSION,
>  				min_t(u8, bytes, SCDC_MIN_SOURCE_VERSION));
>  
>  			/* Enabled Scrambling in the Sink */
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 1);
> +			drm_scdc_set_scrambling(hdmi->ddc, 1);
>  
>  			/*
>  			 * To activate the scrambler feature, you must ensure
> @@ -1705,7 +1709,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL);
>  			hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
>  				    HDMI_MC_SWRSTZ);
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 0);
> +			drm_scdc_set_scrambling(hdmi->ddc, 0);
>  		}
>  	}
>  
> -- 
> 2.17.1
> 
> 




_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Jonas Karlman <jonas@kwiboo.se>
Cc: "dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Archit Taneja <architt@codeaurora.org>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Sean Paul <seanpaul@chromium.org>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/bridge: dw-hdmi: fix SCDC configuration for ddc-i2c-bus
Date: Tue, 23 Apr 2019 12:26:02 +0200	[thread overview]
Message-ID: <2275367.P1g2IIHPGP@phil> (raw)
In-Reply-To: <VE1PR03MB59031814B5BCAB2152923BDAAC210@VE1PR03MB5903.eurprd03.prod.outlook.com>

Am Sonntag, 21. April 2019, 10:25:50 CEST schrieb Jonas Karlman:
> When ddc-i2c-bus property is used, a NULL pointer dereference is reported:
> 
> [   31.041669] Unable to handle kernel NULL pointer dereference at virtual address 00000008
> [   31.041671] pgd = 4d3c16f6
> [   31.041673] [00000008] *pgd=00000000
> [   31.041678] Internal error: Oops: 5 [#1] SMP ARM
> 
> [   31.041711] Hardware name: Rockchip (Device Tree)
> [   31.041718] PC is at i2c_transfer+0x8/0xe4
> [   31.041721] LR is at drm_scdc_read+0x54/0x84
> [   31.041723] pc : [<c073273c>]    lr : [<c05926c4>]    psr: 280f0013
> [   31.041725] sp : edffdad0  ip : 5ccb5511  fp : 00000058
> [   31.041727] r10: 00000780  r9 : edf91608  r8 : c11b0f48
> [   31.041728] r7 : 00000438  r6 : 00000000  r5 : 00000000  r4 : 00000000
> [   31.041730] r3 : edffdae7  r2 : 00000002  r1 : edffdaec  r0 : 00000000
> 
> [   31.041908] [<c073273c>] (i2c_transfer) from [<c05926c4>] (drm_scdc_read+0x54/0x84)
> [   31.041913] [<c05926c4>] (drm_scdc_read) from [<c0592858>] (drm_scdc_set_scrambling+0x30/0xbc)
> [   31.041919] [<c0592858>] (drm_scdc_set_scrambling) from [<c05cc0f4>] (dw_hdmi_update_power+0x1440/0x1610)
> [   31.041926] [<c05cc0f4>] (dw_hdmi_update_power) from [<c05cc574>] (dw_hdmi_bridge_enable+0x2c/0x70)
> [   31.041932] [<c05cc574>] (dw_hdmi_bridge_enable) from [<c05aed48>] (drm_bridge_enable+0x24/0x34)
> [   31.041938] [<c05aed48>] (drm_bridge_enable) from [<c0591060>] (drm_atomic_helper_commit_modeset_enables+0x114/0x220)
> [   31.041943] [<c0591060>] (drm_atomic_helper_commit_modeset_enables) from [<c05c3fe0>] (rockchip_atomic_helper_commit_tail_rpm+0x28/0x64)
> 
> hdmi->i2c may not be set when ddc-i2c-bus property is used in device tree.
> Fix this by using hdmi->ddc as the i2c adapter when calling drm_scdc_*().
> Also report that SCDC is not supported when there is no DDC bus.
> 
> Fixes: 264fce6cc2c1 ("drm/bridge: dw-hdmi: Add SCDC and TMDS Scrambling support")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

I've looked that up and yeah, hdmi->ddc seems the right thing to use
as it points either to the external i2c or the internal one, so

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 7a47c65d8ecb..fd1c319a4ee0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1046,6 +1046,10 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi)
>  	if (hdmi->version < 0x200a)
>  		return false;
>  
> +	/* Disable if no DDC bus */
> +	if (!hdmi->ddc)
> +		return false;
> +
>  	/* Disable if SCDC is not supported, or if an HF-VSDB block is absent */
>  	if (!display->hdmi.scdc.supported ||
>  	    !display->hdmi.scdc.scrambling.supported)
> @@ -1683,13 +1687,13 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			 * Source Devices compliant shall set the
>  			 * Source Version = 1.
>  			 */
> -			drm_scdc_readb(&hdmi->i2c->adap, SCDC_SINK_VERSION,
> +			drm_scdc_readb(hdmi->ddc, SCDC_SINK_VERSION,
>  				       &bytes);
> -			drm_scdc_writeb(&hdmi->i2c->adap, SCDC_SOURCE_VERSION,
> +			drm_scdc_writeb(hdmi->ddc, SCDC_SOURCE_VERSION,
>  				min_t(u8, bytes, SCDC_MIN_SOURCE_VERSION));
>  
>  			/* Enabled Scrambling in the Sink */
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 1);
> +			drm_scdc_set_scrambling(hdmi->ddc, 1);
>  
>  			/*
>  			 * To activate the scrambler feature, you must ensure
> @@ -1705,7 +1709,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL);
>  			hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
>  				    HDMI_MC_SWRSTZ);
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 0);
> +			drm_scdc_set_scrambling(hdmi->ddc, 0);
>  		}
>  	}
>  
> -- 
> 2.17.1
> 
> 





  reply	other threads:[~2019-04-23 10:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20190421082557epcas3p4cf620e148f998076130ded078b2f64be@epcas3p4.samsung.com>
2019-04-21  8:25 ` [PATCH] drm/bridge: dw-hdmi: fix SCDC configuration for ddc-i2c-bus Jonas Karlman
2019-04-21  8:25   ` Jonas Karlman
2019-04-23 10:26   ` Heiko Stuebner [this message]
2019-04-23 10:26     ` Heiko Stuebner
2019-04-23 12:50     ` Laurent Pinchart
2019-04-23 12:47   ` Neil Armstrong
2019-04-25  8:42   ` Andrzej Hajda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2275367.P1g2IIHPGP@phil \
    --to=heiko@sntech.de \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@linux.ie \
    --cc=architt@codeaurora.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=seanpaul@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.