| Message ID | VE1PR03MB59031814B5BCAB2152923BDAAC210@VE1PR03MB5903.eurprd03.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: dw-hdmi: fix SCDC configuration for ddc-i2c-bus | expand |
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 > >
On 21/04/2019 10:25, Jonas Karlman wrote: > 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> > --- > 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); > } > } > > Nice catch ! Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> Thanks, Neil
On Tue, Apr 23, 2019 at 12:26:02PM +0200, Heiko Stuebner wrote: > 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> That matches my own analysis, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > 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); > > } > > } > >
On 21.04.2019 10:25, Jonas Karlman wrote: > 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> Pushed to drm-misc-fixes. Regards Andrzej
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); } }
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> --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)