Hi,
On 03/03/2022 16:37, H. Nikolaus Schaller wrote:
Hi Neil,
Am 03.03.2022 um 12:45 schrieb H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>:
Hi Neil,
Am 03.03.2022 um 12:42 schrieb Neil Armstrong <narmstrong@xxxxxxxxxxxx>:
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.
====================
Fine! Will do.
I was a little too early.
While preparing the patches I found that I still had the hack to force
sink_is_hdmi = false in my test branch. Sorry for that.
Removing this made the panel go black again, even with your latest
proposal.
So I looked deeper into your patch and it seems to influence the
input formats only in dw_hdmi_bridge_atomic_get_input_bus_fmts()?
While the problem I see is with output formats and we had worked on
modifying dw_hdmi_bridge_atomic_get_output_bus_fmts().
I just looked and the ingenic drm driver first bridge uses drm_atomic_helper_bridge_propagate_bus_fmt()
which is why this last patch doesn't work, and perhaps would be the main issue here.
Indeed, the core will loop on all the possible output formats for your HDMI monitor :
- MEDIA_BUS_FMT_UYVY8_1X16
- MEDIA_BUS_FMT_YUV8_1X24
- MEDIA_BUS_FMT_RGB888_1X24
For each of these, the dw-hdmi dw_hdmi_bridge_atomic_get_input_bus_fmts() will
return the same format + the possible CSC transformations, for example
for MEDIA_BUS_FMT_UYVY8_1X16 will return as possible inputs:
- MEDIA_BUS_FMT_UYVY8_1X16
- MEDIA_BUS_FMT_YUV8_1X24
- MEDIA_BUS_FMT_RGB888_1X24
The the core will call for each of the those the .atomic_get_input_bus_fmts of
the Ingenic DRM driver, but by using drm_atomic_helper_bridge_propagate_bus_fmt()
it basically sets a pass-through and accepts any format.
This is why MEDIA_BUS_FMT_UYVY8_1X16 is selected, but in this case the ingenic
ingenic_drm_bridge_atomic_check() would fail in the switch.
The Ingenic should implement a proper .atomic_get_input_bus_fmts returning
only the possible supported formats.
Can you check if you hit the default case in ingenic_drm_bridge_atomic_check() ?
Neil
BR and thanks,
Nikolaus
