lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fca28594-8d4e-dd2f-93a0-a052cb888d90@baylibre.com>
Date:   Wed, 2 Mar 2022 11:25:05 +0100
From:   Neil Armstrong <narmstrong@...libre.com>
To:     "H. Nikolaus Schaller" <hns@...delico.com>
Cc:     Paul Boddie <paul@...die.org.uk>,
        Maxime Ripard <maxime@...no.tech>,
        Jonas Karlman <jonas@...boo.se>,
        David Airlie <airlied@...ux.ie>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-mips <linux-mips@...r.kernel.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Paul Cercueil <paul@...pouillou.net>,
        Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
        Robert Foss <robert.foss@...aro.org>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v16 4/4] drm/bridge: dw-hdmi: fix bus formats negotiation
 for 8 bit modes

H,

On 01/03/2022 21:37, H. Nikolaus Schaller wrote:
> Hi Neil,
> 
> 
>> Am 01.03.2022 um 10:18 schrieb Neil Armstrong <narmstrong@...libre.com>:
>>
>> Hi,
>>
>> On 26/02/2022 18:13, H. Nikolaus Schaller wrote:
>>> Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")
>>> introduced a new mechanism to negotiate bus formats between hdmi connectors
>>> and bridges which is to be used e.g. for the jz4780 based CI20 board.
>>> In this case dw-hdmi sets up a list of formats in
>>> dw_hdmi_bridge_atomic_get_output_bus_fmts().
>>> This includes e.g. MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the CI20 but
>>> only produces a black screen.
>>> Analysis revealed an omission in
>>> Commit 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")
>>> to check for 8 bit with when adding UYVY8 or YUV8 formats.
>>> This fix is based on the observation that max_bpc = 0 when running this
>>> function while info->bpc = 8.
>>
>> In fact if bpc = 0, it should be considered as 8, so the issue is elsewhere.
>>
>>> Adding the proposed patch makes the jz4780/CI20 panel work again with default
>>> MEDIA_BUS_FMT_RGB888_1X24 mode.
>>> Fixes: 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")
>>> Fixes: 6c3c719936dafe ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")
>>> Signed-off-by: H. Nikolaus Schaller <hns@...delico.com>
>>> ---
>>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 10 ++++++----
>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index 43e375da131e8..c08e2cc96584c 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -2621,11 +2621,13 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>>>   		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>>>   	}
>>>   -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>>> -		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>>> +	if (max_bpc >= 8 && info->bpc >= 8) {
>>> +		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>>> +			output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>>>   -	if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>>> -		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>>> +		if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>>> +			output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>>> +	}
>>
>> It should not select YUV here if it's not possible, so something is wrong.
>>
>> Can you check if https://lore.kernel.org/r/20220119123656.1456355-2-narmstrong@baylibre.com fixes this issue instead ?
> 
> Well, I had to manually fix it to be appliable to drm-misc/drm-misc-next
> and specifically:
> 
> c03d0b52ff71 ("drm/connector: Fix typo in output format")
> 
> My resulting patch is attached.
> 
> Unfortunately it did not work.
> 
> I added a printk for hdmi->sink_is_hdmi. This returns 1. Which IMHO is to be expected
> since I am using a HDMI connector and panel... So your patch will still add the UYVY formats.
> 
> Either the synposys module inside the jz4780 or the panel does not understand them.

By selecting the UYVY formats, the driver will enable the colorspace converters in the dw-hdmi IP,
I don't see why it doesn't work here...

There is a bit called `Support Color Space Converter` in config0_id:
bit	|	Name	|	R/W	|	Desc
2 	|	csc	| 	R 	|	Indicates if Color Space Conversion block is present

Could you dump all the config0 bits:

=======================><=============================
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 54d8fdad395f..547731482da8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3431,6 +3431,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
         pdevinfo.id = PLATFORM_DEVID_AUTO;

         config0 = hdmi_readb(hdmi, HDMI_CONFIG0_ID);
+       dev_info(dev, "config0: %x\n", config0);
         config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID);

         if (iores && config3 & HDMI_CONFIG3_AHBAUDDMA) {
=======================><=============================

If this bit is missing, this would explain the black screen.

Neil

> 
> Here is the EDID. Unfortunately it does not pretty print the extended descriptors for UYVY etc.
> so that I don't know the exact capabilities of the panel. And what I am not sure is if the
> jz4780 SoC can convert to UYVY or how it can.
> 
> root@...ux:~# parse-edid </sys/devices/platform/13050000.lcdc0/drm/card0/card0-HDMI-A-1/edid
> Checksum Correct
> 
> Section "Monitor"
>          Identifier "LEN L1950wD"
>          ModelName "LEN L1950wD"
>          VendorName "LEN"
>          # Monitor Manufactured week 34 of 2011
>          # EDID version 1.3
>          # Digital Display
>          DisplaySize 410 260
>          Gamma 2.20
>          Option "DPMS" "true"
>          Horizsync 30-81
>          VertRefresh 50-76
>          # Maximum pixel clock is 140MHz
>          #Not giving standard mode: 1152x864, 75Hz
>          #Not giving standard mode: 1280x720, 60Hz
>          #Not giving standard mode: 1280x1024, 60Hz
>          #Not giving standard mode: 1280x1024, 60Hz
>          #Not giving standard mode: 1280x1024, 60Hz
>          #Not giving standard mode: 1440x900, 60Hz
>          #Not giving standard mode: 1440x900, 75Hz
>          #Not giving standard mode: 1920x1080, 60Hz
> 
>          #Extension block found. Parsing...
>          Modeline        "Mode 15" -hsync -vsync
>          Modeline        "Mode 0" -hsync +vsync
>          Modeline        "Mode 1" 27.027 1440 1478 1602 1716 480 484 487 525 -hsync -vsync interlace
>          Modeline        "Mode 2" 27.027 1440 1478 1602 1716 480 484 487 525 -hsync -vsync interlace
>          Modeline        "Mode 3" 27.027 720 736 798 858 480 489 495 525 -hsync -vsync
>          Modeline        "Mode 4" 27.027 720 736 798 858 480 489 495 525 -hsync -vsync
>          Modeline        "Mode 5" 27.000 1440 1464 1590 1728 576 578 581 625 -hsync -vsync interlace
>          Modeline        "Mode 6" 27.000 1440 1464 1590 1728 576 578 581 625 -hsync -vsync interlace
>          Modeline        "Mode 7" 27.000 720 732 796 864 576 581 586 625 -hsync -vsync
>          Modeline        "Mode 8" 27.000 720 732 796 864 576 581 586 625 -hsync -vsync
>          Modeline        "Mode 9" 74.250 1280 1720 1760 1980 720 725 730 750 +hsync +vsync
>          Modeline        "Mode 10" 74.250 1280 1390 1420 1650 720 725 730 750 +hsync +vsync
>          Modeline        "Mode 11" 74.250 1920 2448 2492 2640 1080 1082 1089 1125 +hsync +vsync interlace
>          Modeline        "Mode 12" 74.250 1920 2008 2052 2200 1080 1082 1087 1125 +hsync +vsync interlace
>          Modeline        "Mode 13" 148.500 1920 2448 2492 2640 1080 1084 1089 1125 +hsync +vsync
>          Modeline        "Mode 14" 148.500 1920 2008 2052 2200 1080 1084 1089 1125 +hsync +vsync
>          Modeline        "Mode 16" +hsync +vsync interlace
>          Modeline        "Mode 17" +hsync +vsync interlace
>          Modeline        "Mode 18" +hsync +vsync
>          Option "PreferredMode" "Mode 15"
> EndSection
> root@...ux:~# xxd /sys/devices/platform/13050000.lcdc0/drm/card0/card0-HDMI-A-1/
> 00000000: 00ff ffff ffff ff00 30ae 8610 0101 0101  ........0.......
> 00000010: 2215 0103 8029 1a78 eee5 b5a3 5549 9927  "....).x....UI.'
> 00000020: 1350 54af ef00 714f 81c0 8180 8180 8180  .PT...qO........
> 00000030: 9500 950f d1c0 2413 0020 4158 1620 050d  ......$.. AX. ..
> 00000040: 2300 ffff 0000 001c 0000 00fc 004c 454e  #............LEN
> 00000050: 204c 3139 3530 7744 0a20 0000 00fd 0032   L1950wD. .....2
> 00000060: 4c1e 510e 000a 2020 2020 2020 0000 00ff  L.Q...      ....
> 00000070: 0042 3334 3332 3834 350a 2020 2020 0101  .B3432845.    ..
> 00000080: 0203 2171 4e06 0702 0315 9611 1213 0414  ..!qN...........
> 00000090: 051f 9023 0907 0783 0100 0065 030c 0010  ...#.......e....
> 000000a0: 008c 0ad0 9020 4031 200c 4055 00b9 8821  ..... @1 .@....!
> 000000b0: 0000 1801 1d80 1871 1c16 2058 2c25 00b9  .......q.. X,%..
> 000000c0: 8821 0000 9e01 1d80 d072 1c16 2010 2c25  .!.......r.. .,%
> 000000d0: 80b9 8821 0000 9e01 1d00 bc52 d01e 20b8  ...!.......R.. .
> 000000e0: 2855 40b9 8821 0000 1e02 3a80 d072 382d  (U@.......:..r8-
> 000000f0: 4010 2c45 80b9 8821 0000 1e00 0000 00d0  @.,E...!........
> root@...ux:~# root@...ux:~# dmesg|grep dw.hdmi
> [    9.622138] dw-hdmi-ingenic 10180000.hdmi: Detected HDMI TX controller v1.31a with HDCP (DWC HDMI 3D TX PHY)
> [    9.727840] dw-hdmi-ingenic 10180000.hdmi: registered DesignWare HDMI I2C bus driver
> [   10.103864] dw_hdmi_bridge_atomic_get_output_bus_fmts: hdmi->sink_is_hdmi=1
> 
> So please let me know which parameters I should try to printk()...
> 
> BR and thanks,
> Nikolaus
> 
> 
> ------
> 
>  From c84a3c4a500684e57b1243fe5386696c48fa1e1b Mon Sep 17 00:00:00 2001
> From: Neil Armstrong <narmstrong@...libre.com>
> Date: Wed, 19 Jan 2022 13:36:56 +0100
> Subject: [PATCH] drm/bridge: dw-hdmi: filter out YUV output formats when DVI
> 
> When the display is not an HDMI sink, only the RGB output format is
> valid. Thus stop returning YUV output formats when sink is not HDMI.
> 
> Fixes: 6c3c719936da ("drm/bridge: synopsys: dw-hdmi: add bus format negociation")
> Signed-off-by: Neil Armstrong <narmstrong@...libre.com>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 43e375da131e8..0ec0cbe448e05 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2538,6 +2538,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>          struct drm_connector *conn = conn_state->connector;
>          struct drm_display_info *info = &conn->display_info;
>          struct drm_display_mode *mode = &crtc_state->mode;
> +       struct dw_hdmi *hdmi = bridge->driver_private;
>          u8 max_bpc = conn_state->max_requested_bpc;
>          bool is_hdmi2_sink = info->hdmi.scdc.supported ||
>                               (info->color_formats & DRM_COLOR_FORMAT_YCBCR420);
> @@ -2564,7 +2565,7 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>           * If the current mode enforces 4:2:0, force the output but format
>           * to 4:2:0 and do not add the YUV422/444/RGB formats
>           */
> -       if (conn->ycbcr_420_allowed &&
> +       if (hdmi->sink_is_hdmi && conn->ycbcr_420_allowed &&
>              (drm_mode_is_420_only(info, mode) ||
>               (is_hdmi2_sink && drm_mode_is_420_also(info, mode)))) {
>   
> @@ -2595,36 +2596,36 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>           */
>   
>          if (max_bpc >= 16 && info->bpc == 16) {
> -               if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> +               if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>                          output_fmts[i++] = MEDIA_BUS_FMT_YUV16_1X48;
>   
>                  output_fmts[i++] = MEDIA_BUS_FMT_RGB161616_1X48;
>          }
>   
>          if (max_bpc >= 12 && info->bpc >= 12) {
> -               if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
> +               if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>                          output_fmts[i++] = MEDIA_BUS_FMT_UYVY12_1X24;
>   
> -               if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> +               if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>                          output_fmts[i++] = MEDIA_BUS_FMT_YUV12_1X36;
>   
>                  output_fmts[i++] = MEDIA_BUS_FMT_RGB121212_1X36;
>          }
>   
>          if (max_bpc >= 10 && info->bpc >= 10) {
> -               if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
> +               if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>                          output_fmts[i++] = MEDIA_BUS_FMT_UYVY10_1X20;
>   
> -               if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> +               if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>                          output_fmts[i++] = MEDIA_BUS_FMT_YUV10_1X30;
>   
>                  output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
>          }
>   
> -       if (info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
> +       if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>                  output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
>   
> -       if (info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> +       if (hdmi->sink_is_hdmi && info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>                  output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
>   
>          /* Default 8bit RGB fallback */

Powered by blists - more mailing lists