Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
On 17/01/2022 15:05, Kieran Bingham wrote: > Quoting Neil Armstrong (2022-01-17 13:53:47) >> On 17/01/2022 11:58, Kieran Bingham wrote: >>> Hi Neil, > > > >>> This fixes the issue for me here on an H3 Salvator-XS. >>> >>> Could you add... >>> >>> Bisected-by: Kieran Bingham >>> Tested-by: Kieran Bingham >>> >>> alongside Biju's Reported-by: tag when posting as a fix please? >> >> >> Which patch did you test ? > > Ah, yes that's not clear is it - sorry! I replied in the wrong place on > the thread when I went back to the mail ... see below... > I'm not happy with this version since it's merely a hack which makes it work. Can you test the following change instead, it's correctly handles your situation in a generic manner. ><= diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 54d8fdad395f..9f2e1cac0ae2 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2551,8 +2551,9 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, if (!output_fmts) return NULL; - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ - if (list_is_singular(>encoder->bridge_chain)) { + /* If dw-hdmi is the first or only bridge, avoid negociating with ourselves */ + if (list_is_singular(>encoder->bridge_chain) || + list_is_first(>chain_node, >encoder->bridge_chain)) { *num_output_fmts = 1; output_fmts[0] = MEDIA_BUS_FMT_FIXED; @@ -2673,6 +2674,10 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, if (!input_fmts) return NULL; + /* If dw-hdmi is the first bridge fall-back to safe output format */ + if (list_is_first(>chain_node, >encoder->bridge_chain)) + output_fmt = MEDIA_BUS_FMT_FIXED; + switch (output_fmt) { /* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */ case MEDIA_BUS_FMT_FIXED: ><= > > Sorry, I replied in the wrong part of the thread. > > Here's the direct diff on my local tree: > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 54d8fdad395f..cac9a87949f1 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2551,8 +2551,10 @@ static u32 > *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, > if (!output_fmts) > return NULL; > > - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ > - if (list_is_singular(>encoder->bridge_chain)) { > + /* If dw-hdmi is the first or only bridge, avoid negociating with > ourselves */ > + if (list_is_singular(>encoder->bridge_chain) || > + list_is_first(>chain_node, > >encoder->bridge_chain)) { > + > *num_output_fmts = 1; > output_fmts[0] = MEDIA_BUS_FMT_FIXED; > > @@ -2673,6 +2675,10 @@ static u32 > *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > if (!input_fmts) > return NULL; > > + /* If dw-hdmi is the first bridge fall-back to safe output format */ > + if (list_is_first(>chain_node, > >encoder->bridge_chain)) > + output_fmt = MEDIA_BUS_FMT_FIXED; > + > switch (output_fmt) { > /* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */ > case MEDIA_BUS_FMT_FIXED: > > Which I believe matches the above. Ok thanks of clarifying ! let me post it asap. Neil > > -- > Kieran >
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Quoting Neil Armstrong (2022-01-17 13:53:47) > On 17/01/2022 11:58, Kieran Bingham wrote: > > Hi Neil, > > This fixes the issue for me here on an H3 Salvator-XS. > > > > Could you add... > > > > Bisected-by: Kieran Bingham > > Tested-by: Kieran Bingham > > > > alongside Biju's Reported-by: tag when posting as a fix please? > > > Which patch did you test ? Ah, yes that's not clear is it - sorry! I replied in the wrong place on the thread when I went back to the mail ... see below... > >> I'm not happy with this version since it's merely a hack which makes it > >> work. > >> > >> Can you test the following change instead, it's correctly handles your > >> situation in a generic manner. > >> > >> ><= > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> index 54d8fdad395f..9f2e1cac0ae2 100644 > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> @@ -2551,8 +2551,9 @@ static u32 > >> *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, > >> if (!output_fmts) > >> return NULL; > >> > >> - /* If dw-hdmi is the only bridge, avoid negociating with ourselves > >> */ > >> - if (list_is_singular(>encoder->bridge_chain)) { > >> + /* If dw-hdmi is the first or only bridge, avoid negociating with > >> ourselves */ > >> + if (list_is_singular(>encoder->bridge_chain) || > >> + list_is_first(>chain_node, > >> >encoder->bridge_chain)) { > >> *num_output_fmts = 1; > >> output_fmts[0] = MEDIA_BUS_FMT_FIXED; > >> > >> @@ -2673,6 +2674,10 @@ static u32 > >> *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > >> if (!input_fmts) > >> return NULL; > >> > >> + /* If dw-hdmi is the first bridge fall-back to safe output format > >> */ > >> + if (list_is_first(>chain_node, > >> >encoder->bridge_chain)) > >> + output_fmt = MEDIA_BUS_FMT_FIXED; > >> + > >> switch (output_fmt) { > >> /* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */ > >> case MEDIA_BUS_FMT_FIXED: > >> ><= Sorry, I replied in the wrong part of the thread. Here's the direct diff on my local tree: diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 54d8fdad395f..cac9a87949f1 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2551,8 +2551,10 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge, if (!output_fmts) return NULL; - /* If dw-hdmi is the only bridge, avoid negociating with ourselves */ - if (list_is_singular(>encoder->bridge_chain)) { + /* If dw-hdmi is the first or only bridge, avoid negociating with ourselves */ + if (list_is_singular(>encoder->bridge_chain) || + list_is_first(>chain_node, >encoder->bridge_chain)) { + *num_output_fmts = 1; output_fmts[0] = MEDIA_BUS_FMT_FIXED; @@ -2673,6 +2675,10 @@ static u32 *dw_hdmi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, if (!input_fmts) return NULL; + /* If dw-hdmi is the first bridge fall-back to safe output format */ + if (list_is_first(>chain_node, >encoder->bridge_chain)) + output_fmt = MEDIA_BUS_FMT_FIXED; + switch (output_fmt) { /* If MEDIA_BUS_FMT_FIXED is tested, return default bus format */ case MEDIA_BUS_FMT_FIXED: Which I believe matches the above. -- Kieran
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
On 17/01/2022 11:58, Kieran Bingham wrote: > Hi Neil, > > Quoting Neil Armstrong (2022-01-17 10:08:38) >> Hi again, >> >> On 14/01/2022 15:40, Neil Armstrong wrote: >>> Hi, >>> >>> On 14/01/2022 15:23, Biju Das wrote: >>>> >>>> >>>>> -Original Message- >>>>> From: Neil Armstrong >>>>> Sent: 14 January 2022 13:56 >>>>> To: Biju Das ; Fabio Estevam >>>>> >>>>> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; >>>>> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; >>>>> martin.blumensti...@googlemail.com; linux-amlo...@lists.infradead.org; >>>>> linux-arm-ker...@lists.infradead.org; dri-devel@lists.freedesktop.org; >>>>> linux-ker...@vger.kernel.org; linux-renesas-...@vger.kernel.org >>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>> callbacks") >>>>> >>>>> Hi, >>>>> >>>>> On 14/01/2022 12:08, Biju Das wrote: >>>>>> Hi Neil, >>>>>> >>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>>>> callbacks") >>>>>>> >>>>>>> On 14/01/2022 09:29, Biju Das wrote: >>>>>>>> Hi Neil, >>>>>>>> >>>>>>>> + renesas-soc >>>>>>>> >>>>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>>>>>> callbacks") >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: >>>>>>>>>> Hi Biju, >>>>>>>>>> >>>>>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >>>>>>>>>> >>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Hi All, >>>>>>>>>>> >>>>>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) >>>>>>>>>>> till the commit >>>>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>>>>>> fmts >>>>>>>>> callbacks"). >>>>>>>>>>> >>>>>>>>>>> After this patch, the screen becomes greenish(may be it is >>>>>>>>>>> setting it >>>>>>>>> into YUV format??). >>>>>>>>>>> >>>>>>>>>>> By checking the code, previously it used to call get_input_fmt >>>>>>>>>>> callback >>>>>>>>> and set colour as RGB24. >>>>>>>>>>> >>>>>>>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 >>>>>>>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, >>>>>>>>>>> I see >>>>>>>>> the outputformat as YUV16 instead of RGB24. >>>>>>>>>>> >>>>>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. >>>>>>>>> >>>>>>>>> This patch was introduced to maintain the bridge color format >>>>>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it >>>>>>>>> seems it behaves incorrectly if the first bridge doesn't implement >>>>>>>>> the negotiation callbacks. >>>>>>>>> >>>>>>>>> Let me check the code to see how to fix that. >>>>>>>> >>>>>>>> Thanks for the information, I am happy to test the patch/fix. >>>>>>>> >>>>>>>> Cheers, >>>>>>>> Biju >>>>>>>> >>>>>>>
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
On 17/01/2022 13:13, Biju Das wrote: > Hi Neil, >> Subject: Re: dw_hdmi is showing wrong colour after commit >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks") >> >> Hi again, >> >> On 14/01/2022 15:40, Neil Armstrong wrote: >>> Hi, >>> >>> On 14/01/2022 15:23, Biju Das wrote: >>>> >>>> >>>>> -Original Message- >>>>> From: Neil Armstrong >>>>> Sent: 14 January 2022 13:56 >>>>> To: Biju Das ; Fabio Estevam >>>>> >>>>> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; >>>>> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; >>>>> martin.blumensti...@googlemail.com; >>>>> linux-amlo...@lists.infradead.org; >>>>> linux-arm-ker...@lists.infradead.org; >>>>> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; >>>>> linux-renesas-...@vger.kernel.org >>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>> callbacks") >>>>> >>>>> Hi, >>>>> >>>>> On 14/01/2022 12:08, Biju Das wrote: >>>>>> Hi Neil, >>>>>> >>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>> fmts >>>>>>> callbacks") >>>>>>> >>>>>>> On 14/01/2022 09:29, Biju Das wrote: >>>>>>>> Hi Neil, >>>>>>>> >>>>>>>> + renesas-soc >>>>>>>> >>>>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>>>> fmts >>>>>>>>> callbacks") >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: >>>>>>>>>> Hi Biju, >>>>>>>>>> >>>>>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >>>>>>>>>> >>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Hi All, >>>>>>>>>>> >>>>>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working >>>>>>>>>>> ok(colour) till the commit >>>>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>>>>>> fmts >>>>>>>>> callbacks"). >>>>>>>>>>> >>>>>>>>>>> After this patch, the screen becomes greenish(may be it is >>>>>>>>>>> setting it >>>>>>>>> into YUV format??). >>>>>>>>>>> >>>>>>>>>>> By checking the code, previously it used to call get_input_fmt >>>>>>>>>>> callback >>>>>>>>> and set colour as RGB24. >>>>>>>>>>> >>>>>>>>>>> After this commit, it calls get_output_fmt_callbck and returns >>>>>>>>>>> 3 outputformats(YUV16, YUV24 and RGB24) And get_input_fmt >>>>>>>>>>> callback, I see >>>>>>>>> the outputformat as YUV16 instead of RGB24. >>>>>>>>>>> >>>>>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI >> driver. >>>>>>>>> >>>>>>>>> This patch was introduced to maintain the bridge color format >>>>>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it >>>>>>>>> seems it behaves incorrectly if the first bridge doesn't >>>>>>>>> implement the negotiation callbacks. >>>>>>>>> >>>>>>>>> Let me check the code to see how to fix that. >>>>>>>> >>>>>>>> Thanks
RE: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi Neil, > Subject: Re: dw_hdmi is showing wrong colour after commit > 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks") > > Hi again, > > On 14/01/2022 15:40, Neil Armstrong wrote: > > Hi, > > > > On 14/01/2022 15:23, Biju Das wrote: > >> > >> > >>> -Original Message- > >>> From: Neil Armstrong > >>> Sent: 14 January 2022 13:56 > >>> To: Biju Das ; Fabio Estevam > >>> > >>> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; > >>> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; > >>> martin.blumensti...@googlemail.com; > >>> linux-amlo...@lists.infradead.org; > >>> linux-arm-ker...@lists.infradead.org; > >>> dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; > >>> linux-renesas-...@vger.kernel.org > >>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >>> callbacks") > >>> > >>> Hi, > >>> > >>> On 14/01/2022 12:08, Biju Das wrote: > >>>> Hi Neil, > >>>> > >>>>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus > >>>>> fmts > >>>>> callbacks") > >>>>> > >>>>> On 14/01/2022 09:29, Biju Das wrote: > >>>>>> Hi Neil, > >>>>>> > >>>>>> + renesas-soc > >>>>>> > >>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus > >>>>>>> fmts > >>>>>>> callbacks") > >>>>>>> > >>>>>>> Hi, > >>>>>>> > >>>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: > >>>>>>>> Hi Biju, > >>>>>>>> > >>>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das > >>>>>>>> > >>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> Hi All, > >>>>>>>>> > >>>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working > >>>>>>>>> ok(colour) till the commit > >>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus > >>>>>>>>> fmts > >>>>>>> callbacks"). > >>>>>>>>> > >>>>>>>>> After this patch, the screen becomes greenish(may be it is > >>>>>>>>> setting it > >>>>>>> into YUV format??). > >>>>>>>>> > >>>>>>>>> By checking the code, previously it used to call get_input_fmt > >>>>>>>>> callback > >>>>>>> and set colour as RGB24. > >>>>>>>>> > >>>>>>>>> After this commit, it calls get_output_fmt_callbck and returns > >>>>>>>>> 3 outputformats(YUV16, YUV24 and RGB24) And get_input_fmt > >>>>>>>>> callback, I see > >>>>>>> the outputformat as YUV16 instead of RGB24. > >>>>>>>>> > >>>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI > driver. > >>>>>>> > >>>>>>> This patch was introduced to maintain the bridge color format > >>>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it > >>>>>>> seems it behaves incorrectly if the first bridge doesn't > >>>>>>> implement the negotiation callbacks. > >>>>>>> > >>>>>>> Let me check the code to see how to fix that. > >>>>>> > >>>>>> Thanks for the information, I am happy to test the patch/fix. > >>>>>> > >>>>>> Cheers, > >>>>>> Biju > >>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> I have tested
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi Neil, Quoting Neil Armstrong (2022-01-17 10:08:38) > Hi again, > > On 14/01/2022 15:40, Neil Armstrong wrote: > > Hi, > > > > On 14/01/2022 15:23, Biju Das wrote: > >> > >> > >>> -Original Message- > >>> From: Neil Armstrong > >>> Sent: 14 January 2022 13:56 > >>> To: Biju Das ; Fabio Estevam > >>> > >>> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; > >>> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; > >>> martin.blumensti...@googlemail.com; linux-amlo...@lists.infradead.org; > >>> linux-arm-ker...@lists.infradead.org; dri-devel@lists.freedesktop.org; > >>> linux-ker...@vger.kernel.org; linux-renesas-...@vger.kernel.org > >>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >>> callbacks") > >>> > >>> Hi, > >>> > >>> On 14/01/2022 12:08, Biju Das wrote: > >>>> Hi Neil, > >>>> > >>>>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >>>>> callbacks") > >>>>> > >>>>> On 14/01/2022 09:29, Biju Das wrote: > >>>>>> Hi Neil, > >>>>>> > >>>>>> + renesas-soc > >>>>>> > >>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >>>>>>> callbacks") > >>>>>>> > >>>>>>> Hi, > >>>>>>> > >>>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: > >>>>>>>> Hi Biju, > >>>>>>>> > >>>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das > >>>>>>>> > >>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> Hi All, > >>>>>>>>> > >>>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) > >>>>>>>>> till the commit > >>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus > >>>>>>>>> fmts > >>>>>>> callbacks"). > >>>>>>>>> > >>>>>>>>> After this patch, the screen becomes greenish(may be it is > >>>>>>>>> setting it > >>>>>>> into YUV format??). > >>>>>>>>> > >>>>>>>>> By checking the code, previously it used to call get_input_fmt > >>>>>>>>> callback > >>>>>>> and set colour as RGB24. > >>>>>>>>> > >>>>>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 > >>>>>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, > >>>>>>>>> I see > >>>>>>> the outputformat as YUV16 instead of RGB24. > >>>>>>>>> > >>>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. > >>>>>>> > >>>>>>> This patch was introduced to maintain the bridge color format > >>>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it > >>>>>>> seems it behaves incorrectly if the first bridge doesn't implement > >>>>>>> the negotiation callbacks. > >>>>>>> > >>>>>>> Let me check the code to see how to fix that. > >>>>>> > >>>>>> Thanks for the information, I am happy to test the patch/fix. > >>>>>> > >>>>>> Cheers, > >>>>>> Biju > >>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> I have tested linux-next 20220112 on a imx6q-sabresd board, which > >>>>> shows: > >>>>>>>> > >>>>>>>> dwh
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi again, On 14/01/2022 15:40, Neil Armstrong wrote: > Hi, > > On 14/01/2022 15:23, Biju Das wrote: >> >> >>> -Original Message- >>> From: Neil Armstrong >>> Sent: 14 January 2022 13:56 >>> To: Biju Das ; Fabio Estevam >>> >>> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; >>> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; >>> martin.blumensti...@googlemail.com; linux-amlo...@lists.infradead.org; >>> linux-arm-ker...@lists.infradead.org; dri-devel@lists.freedesktop.org; >>> linux-ker...@vger.kernel.org; linux-renesas-...@vger.kernel.org >>> Subject: Re: dw_hdmi is showing wrong colour after commit >>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>> callbacks") >>> >>> Hi, >>> >>> On 14/01/2022 12:08, Biju Das wrote: >>>> Hi Neil, >>>> >>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>> callbacks") >>>>> >>>>> On 14/01/2022 09:29, Biju Das wrote: >>>>>> Hi Neil, >>>>>> >>>>>> + renesas-soc >>>>>> >>>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>>>> callbacks") >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: >>>>>>>> Hi Biju, >>>>>>>> >>>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >>>>>>>> >>>>>>> wrote: >>>>>>>>> >>>>>>>>> Hi All, >>>>>>>>> >>>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) >>>>>>>>> till the commit >>>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>>>> fmts >>>>>>> callbacks"). >>>>>>>>> >>>>>>>>> After this patch, the screen becomes greenish(may be it is >>>>>>>>> setting it >>>>>>> into YUV format??). >>>>>>>>> >>>>>>>>> By checking the code, previously it used to call get_input_fmt >>>>>>>>> callback >>>>>>> and set colour as RGB24. >>>>>>>>> >>>>>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 >>>>>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, >>>>>>>>> I see >>>>>>> the outputformat as YUV16 instead of RGB24. >>>>>>>>> >>>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. >>>>>>> >>>>>>> This patch was introduced to maintain the bridge color format >>>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it >>>>>>> seems it behaves incorrectly if the first bridge doesn't implement >>>>>>> the negotiation callbacks. >>>>>>> >>>>>>> Let me check the code to see how to fix that. >>>>>> >>>>>> Thanks for the information, I am happy to test the patch/fix. >>>>>> >>>>>> Cheers, >>>>>> Biju >>>>>> >>>>>>> >>>>>>>> >>>>>>>> I have tested linux-next 20220112 on a imx6q-sabresd board, which >>>>> shows: >>>>>>>> >>>>>>>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with >>>>>>>> HDCP (DWC HDMI 3D TX PHY) >>>>>>>> >>>>>>>> The colors are shown correctly here. >>>>>>>> >>>>>>> >>>>>>> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the >>>>>>> negotiation fails and use the RGB fallback input & output format. >>>>>>> >>>>>>> Anyway thanks for testing
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi, On 14/01/2022 15:23, Biju Das wrote: > > >> -Original Message- >> From: Neil Armstrong >> Sent: 14 January 2022 13:56 >> To: Biju Das ; Fabio Estevam >> >> Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; >> robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; >> martin.blumensti...@googlemail.com; linux-amlo...@lists.infradead.org; >> linux-arm-ker...@lists.infradead.org; dri-devel@lists.freedesktop.org; >> linux-ker...@vger.kernel.org; linux-renesas-...@vger.kernel.org >> Subject: Re: dw_hdmi is showing wrong colour after commit >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks") >> >> Hi, >> >> On 14/01/2022 12:08, Biju Das wrote: >>> Hi Neil, >>> >>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>> callbacks") >>>> >>>> On 14/01/2022 09:29, Biju Das wrote: >>>>> Hi Neil, >>>>> >>>>> + renesas-soc >>>>> >>>>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>>>> callbacks") >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 13/01/2022 21:01, Fabio Estevam wrote: >>>>>>> Hi Biju, >>>>>>> >>>>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >>>>>>> >>>>>> wrote: >>>>>>>> >>>>>>>> Hi All, >>>>>>>> >>>>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) >>>>>>>> till the commit >>>>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus >>>>>>>> fmts >>>>>> callbacks"). >>>>>>>> >>>>>>>> After this patch, the screen becomes greenish(may be it is >>>>>>>> setting it >>>>>> into YUV format??). >>>>>>>> >>>>>>>> By checking the code, previously it used to call get_input_fmt >>>>>>>> callback >>>>>> and set colour as RGB24. >>>>>>>> >>>>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 >>>>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, >>>>>>>> I see >>>>>> the outputformat as YUV16 instead of RGB24. >>>>>>>> >>>>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. >>>>>> >>>>>> This patch was introduced to maintain the bridge color format >>>>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it >>>>>> seems it behaves incorrectly if the first bridge doesn't implement >>>>>> the negotiation callbacks. >>>>>> >>>>>> Let me check the code to see how to fix that. >>>>> >>>>> Thanks for the information, I am happy to test the patch/fix. >>>>> >>>>> Cheers, >>>>> Biju >>>>> >>>>>> >>>>>>> >>>>>>> I have tested linux-next 20220112 on a imx6q-sabresd board, which >>>> shows: >>>>>>> >>>>>>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with >>>>>>> HDCP (DWC HDMI 3D TX PHY) >>>>>>> >>>>>>> The colors are shown correctly here. >>>>>>> >>>>>> >>>>>> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the >>>>>> negotiation fails and use the RGB fallback input & output format. >>>>>> >>>>>> Anyway thanks for testing >>>>>> >>>>>> Neil >>>> >>>> Can you test : >>>> >>>> ==><=== >>>> diff --git a/drivers/gpu/drm/drm_bridge.c >>>> b/drivers/gpu/drm/drm_bridge.c index c96847fc0ebc..7019acd37716 >>>> 100644 >>>> --- a/drivers/gpu/drm/drm_bridge.c >>>> +++ b/drivers/gpu/drm/drm_bridge.c &g
RE: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
> -Original Message- > From: Neil Armstrong > Sent: 14 January 2022 13:56 > To: Biju Das ; Fabio Estevam > > Cc: dan...@ffwll.ch; laurent.pinch...@ideasonboard.com; > robert.f...@linaro.org; jo...@kwiboo.se; jernej.skra...@gmail.com; > martin.blumensti...@googlemail.com; linux-amlo...@lists.infradead.org; > linux-arm-ker...@lists.infradead.org; dri-devel@lists.freedesktop.org; > linux-ker...@vger.kernel.org; linux-renesas-...@vger.kernel.org > Subject: Re: dw_hdmi is showing wrong colour after commit > 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks") > > Hi, > > On 14/01/2022 12:08, Biju Das wrote: > > Hi Neil, > > > >> Subject: Re: dw_hdmi is showing wrong colour after commit > >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >> callbacks") > >> > >> On 14/01/2022 09:29, Biju Das wrote: > >>> Hi Neil, > >>> > >>> + renesas-soc > >>> > >>>> Subject: Re: dw_hdmi is showing wrong colour after commit > >>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >>>> callbacks") > >>>> > >>>> Hi, > >>>> > >>>> On 13/01/2022 21:01, Fabio Estevam wrote: > >>>>> Hi Biju, > >>>>> > >>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das > >>>>> > >>>> wrote: > >>>>>> > >>>>>> Hi All, > >>>>>> > >>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) > >>>>>> till the commit > >>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus > >>>>>> fmts > >>>> callbacks"). > >>>>>> > >>>>>> After this patch, the screen becomes greenish(may be it is > >>>>>> setting it > >>>> into YUV format??). > >>>>>> > >>>>>> By checking the code, previously it used to call get_input_fmt > >>>>>> callback > >>>> and set colour as RGB24. > >>>>>> > >>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 > >>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, > >>>>>> I see > >>>> the outputformat as YUV16 instead of RGB24. > >>>>>> > >>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. > >>>> > >>>> This patch was introduced to maintain the bridge color format > >>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it > >>>> seems it behaves incorrectly if the first bridge doesn't implement > >>>> the negotiation callbacks. > >>>> > >>>> Let me check the code to see how to fix that. > >>> > >>> Thanks for the information, I am happy to test the patch/fix. > >>> > >>> Cheers, > >>> Biju > >>> > >>>> > >>>>> > >>>>> I have tested linux-next 20220112 on a imx6q-sabresd board, which > >> shows: > >>>>> > >>>>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with > >>>>> HDCP (DWC HDMI 3D TX PHY) > >>>>> > >>>>> The colors are shown correctly here. > >>>>> > >>>> > >>>> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the > >>>> negotiation fails and use the RGB fallback input & output format. > >>>> > >>>> Anyway thanks for testing > >>>> > >>>> Neil > >> > >> Can you test : > >> > >> ==><=== > >> diff --git a/drivers/gpu/drm/drm_bridge.c > >> b/drivers/gpu/drm/drm_bridge.c index c96847fc0ebc..7019acd37716 > >> 100644 > >> --- a/drivers/gpu/drm/drm_bridge.c > >> +++ b/drivers/gpu/drm/drm_bridge.c > >> @@ -955,7 +955,14 @@ drm_atomic_bridge_chain_select_bus_fmts(struct > >> drm_bridge *bridge, > >> last_bridge_state = > >> drm_atomic_get_new_bridge_state(crtc_state- > >>> state, > >> > >> last_bridge); > >> > >> - if (last_bridge->funcs->atomic_get_output_bus_f
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi, On 14/01/2022 12:08, Biju Das wrote: > Hi Neil, > >> Subject: Re: dw_hdmi is showing wrong colour after commit >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks") >> >> On 14/01/2022 09:29, Biju Das wrote: >>> Hi Neil, >>> >>> + renesas-soc >>> >>>> Subject: Re: dw_hdmi is showing wrong colour after commit >>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>> callbacks") >>>> >>>> Hi, >>>> >>>> On 13/01/2022 21:01, Fabio Estevam wrote: >>>>> Hi Biju, >>>>> >>>>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >>>>> >>>> wrote: >>>>>> >>>>>> Hi All, >>>>>> >>>>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) >>>>>> till the commit >>>>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >>>> callbacks"). >>>>>> >>>>>> After this patch, the screen becomes greenish(may be it is setting >>>>>> it >>>> into YUV format??). >>>>>> >>>>>> By checking the code, previously it used to call get_input_fmt >>>>>> callback >>>> and set colour as RGB24. >>>>>> >>>>>> After this commit, it calls get_output_fmt_callbck and returns 3 >>>>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, I >>>>>> see >>>> the outputformat as YUV16 instead of RGB24. >>>>>> >>>>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. >>>> >>>> This patch was introduced to maintain the bridge color format >>>> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems >>>> it behaves incorrectly if the first bridge doesn't implement the >>>> negotiation callbacks. >>>> >>>> Let me check the code to see how to fix that. >>> >>> Thanks for the information, I am happy to test the patch/fix. >>> >>> Cheers, >>> Biju >>> >>>> >>>>> >>>>> I have tested linux-next 20220112 on a imx6q-sabresd board, which >> shows: >>>>> >>>>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP >>>>> (DWC HDMI 3D TX PHY) >>>>> >>>>> The colors are shown correctly here. >>>>> >>>> >>>> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation >>>> fails and use the RGB fallback input & output format. >>>> >>>> Anyway thanks for testing >>>> >>>> Neil >> >> Can you test : >> >> ==><=== >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >> index c96847fc0ebc..7019acd37716 100644 >> --- a/drivers/gpu/drm/drm_bridge.c >> +++ b/drivers/gpu/drm/drm_bridge.c >> @@ -955,7 +955,14 @@ drm_atomic_bridge_chain_select_bus_fmts(struct >> drm_bridge *bridge, >> last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state- >>> state, >> last_bridge); >> >> - if (last_bridge->funcs->atomic_get_output_bus_fmts) { >> + /* >> +* Only negociate with real values if both end of the bridge chain >> +* support negociation callbacks, otherwise you can end in a >> situation >> +* where the selected output format doesn't match with the first >> bridge >> +* output format. >> +*/ >> + if (bridge->funcs->atomic_get_input_bus_fmts && >> + last_bridge->funcs->atomic_get_output_bus_fmts) { >> const struct drm_bridge_funcs *funcs = last_bridge->funcs; >> >> /* >> @@ -980,7 +987,12 @@ drm_atomic_bridge_chain_select_bus_fmts(struct >> drm_bridge *bridge, >> if (!out_bus_fmts) >> return -ENOMEM; >> >> - if (conn->display_info.num_bus_formats && >> + /* >> +* If first bridge doesn't support negociation, use >> MEDIA_BUS_FMT_FIXED >> +* as a safe value for the whole bridge chain &
RE: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi Neil, > Subject: Re: dw_hdmi is showing wrong colour after commit > 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks") > > On 14/01/2022 09:29, Biju Das wrote: > > Hi Neil, > > > > + renesas-soc > > > >> Subject: Re: dw_hdmi is showing wrong colour after commit > >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >> callbacks") > >> > >> Hi, > >> > >> On 13/01/2022 21:01, Fabio Estevam wrote: > >>> Hi Biju, > >>> > >>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das > >>> > >> wrote: > >>>> > >>>> Hi All, > >>>> > >>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) > >>>> till the commit > >>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > >> callbacks"). > >>>> > >>>> After this patch, the screen becomes greenish(may be it is setting > >>>> it > >> into YUV format??). > >>>> > >>>> By checking the code, previously it used to call get_input_fmt > >>>> callback > >> and set colour as RGB24. > >>>> > >>>> After this commit, it calls get_output_fmt_callbck and returns 3 > >>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, I > >>>> see > >> the outputformat as YUV16 instead of RGB24. > >>>> > >>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. > >> > >> This patch was introduced to maintain the bridge color format > >> negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems > >> it behaves incorrectly if the first bridge doesn't implement the > >> negotiation callbacks. > >> > >> Let me check the code to see how to fix that. > > > > Thanks for the information, I am happy to test the patch/fix. > > > > Cheers, > > Biju > > > >> > >>> > >>> I have tested linux-next 20220112 on a imx6q-sabresd board, which > shows: > >>> > >>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP > >>> (DWC HDMI 3D TX PHY) > >>> > >>> The colors are shown correctly here. > >>> > >> > >> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation > >> fails and use the RGB fallback input & output format. > >> > >> Anyway thanks for testing > >> > >> Neil > > Can you test : > > ==><=== > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index c96847fc0ebc..7019acd37716 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -955,7 +955,14 @@ drm_atomic_bridge_chain_select_bus_fmts(struct > drm_bridge *bridge, > last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state- > >state, > last_bridge); > > - if (last_bridge->funcs->atomic_get_output_bus_fmts) { > + /* > +* Only negociate with real values if both end of the bridge chain > +* support negociation callbacks, otherwise you can end in a > situation > +* where the selected output format doesn't match with the first > bridge > +* output format. > +*/ > + if (bridge->funcs->atomic_get_input_bus_fmts && > + last_bridge->funcs->atomic_get_output_bus_fmts) { > const struct drm_bridge_funcs *funcs = last_bridge->funcs; > > /* > @@ -980,7 +987,12 @@ drm_atomic_bridge_chain_select_bus_fmts(struct > drm_bridge *bridge, > if (!out_bus_fmts) > return -ENOMEM; > > - if (conn->display_info.num_bus_formats && > + /* > +* If first bridge doesn't support negociation, use > MEDIA_BUS_FMT_FIXED > +* as a safe value for the whole bridge chain > +*/ > + if (bridge->funcs->atomic_get_input_bus_fmts && > + conn->display_info.num_bus_formats && > conn->display_info.bus_formats) > out_bus_fmts[0] = conn- > >display_info.bus_formats[0]; > else > ==><=== > > This should exclude your situation where the first bridge doesn't support > negociation. I have tested this fix with Linux next-20220114. Still I see colour issue. It is still negotiating and it is calling get_output_fmt_callbck [3.460155] dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_UYVY8_1X16=0# [3.460180] dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_YUV8_1X24=1# [3.460202] dw_hdmi_bridge_atomic_get_output_bus_fmts MEDIA_BUS_FMT_RGB888_1X24=2# And In get_input_fmt callback, I See the outputformat as YUV16 instead of RGB24. [3.460319] dw_hdmi_bridge_atomic_get_input_bus_fmts MEDIA_BUS_FMT_UYVY8_1X16# [3.473644] hdmi_video_sample MEDIA_BUS_FMT_UYVY8_1X16# Regards, Biju
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
On 14/01/2022 09:29, Biju Das wrote: > Hi Neil, > > + renesas-soc > >> Subject: Re: dw_hdmi is showing wrong colour after commit >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks") >> >> Hi, >> >> On 13/01/2022 21:01, Fabio Estevam wrote: >>> Hi Biju, >>> >>> On Thu, Jan 13, 2022 at 2:45 PM Biju Das >> wrote: >>>> >>>> Hi All, >>>> >>>> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) till >>>> the commit >>>> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks"). >>>> >>>> After this patch, the screen becomes greenish(may be it is setting it >> into YUV format??). >>>> >>>> By checking the code, previously it used to call get_input_fmt callback >> and set colour as RGB24. >>>> >>>> After this commit, it calls get_output_fmt_callbck and returns 3 >>>> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, I see >> the outputformat as YUV16 instead of RGB24. >>>> >>>> Not sure, I am the only one seeing this issue with dw_HDMI driver. >> >> This patch was introduced to maintain the bridge color format negotiation >> after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems it behaves >> incorrectly if the first bridge doesn't implement the negotiation >> callbacks. >> >> Let me check the code to see how to fix that. > > Thanks for the information, I am happy to test the patch/fix. > > Cheers, > Biju > >> >>> >>> I have tested linux-next 20220112 on a imx6q-sabresd board, which shows: >>> >>> dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP >>> (DWC HDMI 3D TX PHY) >>> >>> The colors are shown correctly here. >>> >> >> The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation >> fails and use the RGB fallback input & output format. >> >> Anyway thanks for testing >> >> Neil Can you test : ==><=== diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c96847fc0ebc..7019acd37716 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -955,7 +955,14 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge, last_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state, last_bridge); - if (last_bridge->funcs->atomic_get_output_bus_fmts) { + /* +* Only negociate with real values if both end of the bridge chain +* support negociation callbacks, otherwise you can end in a situation +* where the selected output format doesn't match with the first bridge +* output format. +*/ + if (bridge->funcs->atomic_get_input_bus_fmts && + last_bridge->funcs->atomic_get_output_bus_fmts) { const struct drm_bridge_funcs *funcs = last_bridge->funcs; /* @@ -980,7 +987,12 @@ drm_atomic_bridge_chain_select_bus_fmts(struct drm_bridge *bridge, if (!out_bus_fmts) return -ENOMEM; - if (conn->display_info.num_bus_formats && + /* +* If first bridge doesn't support negociation, use MEDIA_BUS_FMT_FIXED +* as a safe value for the whole bridge chain +*/ + if (bridge->funcs->atomic_get_input_bus_fmts && + conn->display_info.num_bus_formats && conn->display_info.bus_formats) out_bus_fmts[0] = conn->display_info.bus_formats[0]; else ==><=== This should exclude your situation where the first bridge doesn't support negociation. Neil
RE: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi Neil, + renesas-soc > Subject: Re: dw_hdmi is showing wrong colour after commit > 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks") > > Hi, > > On 13/01/2022 21:01, Fabio Estevam wrote: > > Hi Biju, > > > > On Thu, Jan 13, 2022 at 2:45 PM Biju Das > wrote: > >> > >> Hi All, > >> > >> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) till > >> the commit > >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks"). > >> > >> After this patch, the screen becomes greenish(may be it is setting it > into YUV format??). > >> > >> By checking the code, previously it used to call get_input_fmt callback > and set colour as RGB24. > >> > >> After this commit, it calls get_output_fmt_callbck and returns 3 > >> outputformats(YUV16, YUV24 and RGB24) And get_input_fmt callback, I see > the outputformat as YUV16 instead of RGB24. > >> > >> Not sure, I am the only one seeing this issue with dw_HDMI driver. > > This patch was introduced to maintain the bridge color format negotiation > after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems it behaves > incorrectly if the first bridge doesn't implement the negotiation > callbacks. > > Let me check the code to see how to fix that. Thanks for the information, I am happy to test the patch/fix. Cheers, Biju > > > > > I have tested linux-next 20220112 on a imx6q-sabresd board, which shows: > > > > dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP > > (DWC HDMI 3D TX PHY) > > > > The colors are shown correctly here. > > > > The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation > fails and use the RGB fallback input & output format. > > Anyway thanks for testing > > Neil
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi, On 13/01/2022 21:01, Fabio Estevam wrote: > Hi Biju, > > On Thu, Jan 13, 2022 at 2:45 PM Biju Das wrote: >> >> Hi All, >> >> RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) till the >> commit >> 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts >> callbacks"). >> >> After this patch, the screen becomes greenish(may be it is setting it into >> YUV format??). >> >> By checking the code, previously it used to call get_input_fmt callback and >> set colour as RGB24. >> >> After this commit, it calls get_output_fmt_callbck and returns 3 >> outputformats(YUV16, YUV24 and RGB24) >> And get_input_fmt callback, I see the outputformat as YUV16 instead of RGB24. >> >> Not sure, I am the only one seeing this issue with dw_HDMI driver. This patch was introduced to maintain the bridge color format negotiation after using DRM_BRIDGE_ATTACH_NO_CONNECTOR, but it seems it behaves incorrectly if the first bridge doesn't implement the negotiation callbacks. Let me check the code to see how to fix that. > > I have tested linux-next 20220112 on a imx6q-sabresd board, which shows: > > dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP > (DWC HDMI 3D TX PHY) > > The colors are shown correctly here. > The imx doesn't use DRM_BRIDGE_ATTACH_NO_CONNECTOR so the negotiation fails and use the RGB fallback input & output format. Anyway thanks for testing Neil
Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")
Hi Biju, On Thu, Jan 13, 2022 at 2:45 PM Biju Das wrote: > > Hi All, > > RZ/G2{H, M, N} SoC has dw_hdmi IP and it was working ok(colour) till the > commit > 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts > callbacks"). > > After this patch, the screen becomes greenish(may be it is setting it into > YUV format??). > > By checking the code, previously it used to call get_input_fmt callback and > set colour as RGB24. > > After this commit, it calls get_output_fmt_callbck and returns 3 > outputformats(YUV16, YUV24 and RGB24) > And get_input_fmt callback, I see the outputformat as YUV16 instead of RGB24. > > Not sure, I am the only one seeing this issue with dw_HDMI driver. I have tested linux-next 20220112 on a imx6q-sabresd board, which shows: dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP (DWC HDMI 3D TX PHY) The colors are shown correctly here.