Re: dw_hdmi is showing wrong colour after commit 7cd70656d1285b79("drm/bridge: display-connector: implement bus fmts callbacks")

2022-01-17 Thread Neil Armstrong
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")

2022-01-17 Thread Kieran Bingham
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")

2022-01-17 Thread Neil Armstrong
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")

2022-01-17 Thread Neil Armstrong
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")

2022-01-17 Thread Biju Das
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")

2022-01-17 Thread Kieran Bingham
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")

2022-01-17 Thread Neil Armstrong
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")

2022-01-14 Thread Neil Armstrong
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")

2022-01-14 Thread Biju Das


> -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")

2022-01-14 Thread Neil Armstrong
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")

2022-01-14 Thread Biju Das
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")

2022-01-14 Thread Neil Armstrong
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")

2022-01-14 Thread Biju Das
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")

2022-01-14 Thread Neil Armstrong
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")

2022-01-13 Thread Fabio Estevam
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.