Re: Re: [PATCH v5 15/44] drm/connector: hdmi: Compute bpc and format automatically

[Maxime Ripard]

On Thu, Dec 14, 2023 at 03:10:43PM +0000, Dave Stevenson wrote:
> > +static bool
> > +sink_supports_format_bpc(const struct drm_connector *connector,
> > +                        const struct drm_display_info *info,
> > +                        const struct drm_display_mode *mode,
> > +                        unsigned int format, unsigned int bpc)
> > +{
> > +       struct drm_device *dev = connector->dev;
> > +       u8 vic = drm_match_cea_mode(mode);
> > +
> > +       if (vic == 1 && bpc != 8) {
> > +               drm_dbg(dev, "VIC1 requires a bpc of 8, got %u\n", bpc);
> > +               return false;
> > +       }
> > +
> > +       if (!info->is_hdmi &&
> > +           (format != HDMI_COLORSPACE_RGB || bpc != 8)) {
> > +               drm_dbg(dev, "DVI Monitors require an RGB output at 8 bpc\n");
> > +               return false;
> > +       }
> > +
> > +       if (!(connector->hdmi.supported_formats & BIT(format))) {
> > +               drm_dbg(dev, "%s format unsupported by the connector.\n",
> > +                       drm_hdmi_connector_get_output_format_name(format));
> > +               return false;
> > +       }
> > +
> > +       switch (format) {
> > +       case HDMI_COLORSPACE_RGB:
> > +               drm_dbg(dev, "RGB Format, checking the constraints.\n");
> > +
> > +               if (!(info->color_formats & DRM_COLOR_FORMAT_RGB444))
> > +                       return false;
> 
> We've dropped this check from vc4 in our downstream kernel as it stops
> you using the prebaked EDIDs (eg drm.edid_firmware=edid/1024x768.bin),
> or any other EDID that is defined as an analog monitor.
> The EDID parsing bombs out at [1], so info->color_formats gets left at 0.

Right, but it only does so if the display isn't defined as a digital display...

> RGB is mandatory for both DVI and HDMI, so rejecting it seems overly fussy.

... which is required for both DVI and HDMI.

And sure enough, if we decode that EDID:

edid-decode (hex):

00 ff ff ff ff ff ff 00 31 d8 00 00 00 00 00 00
05 16 01 03 6d 23 1a 78 ea 5e c0 a4 59 4a 98 25
20 50 54 00 08 00 61 40 01 01 01 01 01 01 01 01
01 01 01 01 01 01 64 19 00 40 41 00 26 30 08 90
36 00 63 0a 11 00 00 18 00 00 00 ff 00 4c 69 6e
75 78 20 23 30 0a 20 20 20 20 00 00 00 fd 00 3b
3d 2f 31 07 00 0a 20 20 20 20 20 20 00 00 00 fc
00 4c 69 6e 75 78 20 58 47 41 0a 20 20 20 00 55

----------------

Block 0, Base EDID:
  EDID Structure Version & Revision: 1.3
  Vendor & Product Identification:
    Manufacturer: LNX
    Model: 0
    Made in: week 5 of 2012
  Basic Display Parameters & Features:
    Analog display
    Signal Level Standard: 0.700 : 0.000 : 0.700 V p-p
    Blank level equals black level
    Sync: Separate Composite Serration
    Maximum image size: 35 cm x 26 cm
    Gamma: 2.20
    DPMS levels: Standby Suspend Off
    RGB color display
    First detailed timing is the preferred timing
  Color Characteristics:
    Red  : 0.6416, 0.3486
    Green: 0.2919, 0.5957
    Blue : 0.1474, 0.1250
    White: 0.3125, 0.3281
  Established Timings I & II:
    DMT 0x10:  1024x768    60.003840 Hz   4:3     48.363 kHz     65.000000 MHz
  Standard Timings:
    DMT 0x10:  1024x768    60.003840 Hz   4:3     48.363 kHz     65.000000 MHz
  Detailed Timing Descriptors:
    DTD 1:  1024x768    60.003840 Hz   4:3     48.363 kHz     65.000000 MHz (355 mm x 266 mm)
                 Hfront    8 Hsync 144 Hback  168 Hpol N
                 Vfront    3 Vsync   6 Vback   29 Vpol N
    Display Product Serial Number: 'Linux #0'
    Display Range Limits:
      Monitor ranges (GTF): 59-61 Hz V, 47-49 kHz H, max dotclock 70 MHz
    Display Product Name: 'Linux XGA'
Checksum: 0x55

----------------

Warnings:

Block 0, Base EDID:
  Detailed Timing Descriptor #1: DTD is similar but not identical to DMT 0x10.

EDID conformity: PASS

So, if anything, it's the EDID that needs to be updated, not the code there.
Maxime

Attachment: signature.asc
Description: PGP signature