Re: [PATCH v16 4/4] drm/bridge: dw-hdmi: fix bus formats negotiation for 8 bit modes

[Neil Armstrong <narmstrong@baylibre.com>]

On 03/03/2022 11:40, H. Nikolaus Schaller wrote:
> Hi Neil,
> 
>> Am 03.03.2022 um 09:35 schrieb Neil Armstrong <narmstrong@baylibre.com>:
>>
>> Hi,
>>
>> On 02/03/2022 23:24, H. Nikolaus Schaller wrote:
>>> Hi Neil,
>>>> Am 02.03.2022 um 15:34 schrieb Neil Armstrong <narmstrong@baylibre.com>:
>>>>
>>>> Hi,
>>>>
>>>>> (cross-checked: RGB mode still works if I force hdmi->sink_is_hdmi = false)
>>>>
>>>> I don't understand what's wrong, can you try to make the logic select MEDIA_BUS_FMT_YUV8_1X24 instead of DRM_COLOR_FORMAT_YCBCR422 ?
>>> I have forced hdmi->sink_is_hdmi = false and replaced
>>>   	/* Default 8bit RGB fallback */
>>> -	output_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
>>> +	output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>>> And then screen remains black. MEDIA_BUS_FMT_RGB888_1X24 works.
>>> (MEDIA_BUS_FMT_VUY8_1X24 doesn't work either).
>>> So this indicates that YUV conversion is not working properly. Maybe missing some special
>>> setup.
>>> What I have to test if it works on a different monitor.
> 
> Same effect on a Xiaomi monitor (user manual just telling HDMI1,4 compatible), an
> older Acer video projector and a Sharp TV set.
> 
> The Xiaomi monitor does not say "No signal" but shows a black screen. The others
> do not even report any HDMI signals. All work well with MEDIA_BUS_FMT_RGB888_1X24.
> 
> This means the transcoding to YUV does not work properly on the jz4780 SoC setup.
> 
> So it looks as if we have to disable it (at least unless someone finds a fix).
> 
>>> Not that this specific panel
>>> (a 7 inch waveshare touch with HDMIinput) is buggy and reports YUV capabilities
>>> but does not handle them...
>>> On the other hand this panel works on RasPi and OMAP5 (where I admit I do not know in
>>> which mode).
>>
>> Pretty sure they don't support YUV HDMI output.
>>
>> If you can try on a certified HDMI devices like a TV, it would here figuring out where comes the issue.
> 
> I am not sure if the Sharp TV is fully certified but would assume...
> 
>>
>>>> If your CSC is broken, we'll need to disable it on your platform.
>>> Indeed.
>>> So it seems as if we need a mechanism to overwrite dw_hdmi_bridge_atomic_get_output_bus_fmts()
>>> in our ingenic-dw-hdmi platform specialization [1] to always return MEDIA_BUS_FMT_RGB888_1X24.
>>> Or alternatively set sink_is_hdmi = false there (unfortunately there is no direct access to
>>> struct dw_hdmi in a specialization drivers).
>>> Is this already possible or how can it be done?
>>
>> It's not handled yet, but we may add the logic to handle the lack of CSC config bit and
>> add a glue config bit to override this like we already did for CEC.
>>
>> I wrote an initial support to disable CSC (only compile-tested), could you try on your platform with setting disable_csc = 1 in your dw-hdmi glue code ?
> 
> This works!
> 
> So how can we get that merged? IMHO your proposal should be before we add ingenic-dw-hdmi.
> If you have a version with proper commit message I can add it to the beginning of my
> seried and include it in a v17. Or if you get yours merged to drm-misc/drm-misc-next I
> can build on top.

You can add it in your v17 patchset with my authorship and my Signed-off-by tag + yours.

As commit message something like :
====================
drm/bridge: dw-hdmi: handle unusable or non-configured CSC module

The dw-hdmi integrates an optional Color Space Conversion feature used
to handle color-space conversions.

On some platforms, the CSC isn't built-in or non-functional.

This adds the necessary code to disable the CSC functionality
and limit the bus format negotiation to force using the same
input bus format as the output bus format.
====================

Thanks,
Neil

> 
> BR and thanks,
> Nikolaus
>