RE: [PATCH v2] drm/ast: DisplayPort edid supports 256 bytes

[Jammy Huang]

Hi Thomas,

I'd like to get your thoughts on Jani's point that the kernel should avoid modifying EDID and instead expose the data as-is to userspace.

In practice, most monitors I have encountered only contain two EDID blocks, so simply returning block 0 and block 1 already works for the majority of cases.

Given that, would you agree that it may be acceptable to stop modifying the EDID in the driver and only support the blocks the hardware can actually read now?

Best regards,
Jammy

> 
> Hi Jani,
> 
> Thanks for your input. So, your suggestion is go back to return -EIO for block >
> 1, and stop trying to modify the EDID.
> In this way, we can handle EDID whose blocks is less than 2.
> 
> BR,
> Jammy
> 
> > > Hi Thomas,
> > >
> > > Good catch - returning 0 without writing to buf wasn't safe. But I
> > > also found that returning -EIO isn't right either:
> > > _drm_do_get_edid() treats EDID_BLOCK_READ_FAIL as fatal for any
> > > block index, not just block 0, so that would have discarded the
> > > whole EDID for any monitor reporting more than one extension.
> > >
> > > In v3, for block > 1 I now zero the buffer explicitly and return 0.
> > > That makes drm_edid's block checker classify it as EDID_BLOCK_ZERO
> > > instead, which is non-fatal - it gets trimmed via
> > > edid_filter_invalid_blocks() (extensions count reduced, checksum
> > > recomputed), and blocks 0 and 1 still come through fine. This also
> > > directly addresses your point about the buffer not being guaranteed
> > > zeroed, since we zero it ourselves now.
> >
> > There's a long-term goal to *not* modify the EDID in kernel, but
> > rather return even the broken EDID extensions to userspace, if that's what
> the display has.
> >
> > There are probably a bunch of hurdles in making that happen, but
> > intentionally returning zeroed out EDID blocks is just adding another
> unnecessary hurdle.
> >
> >
> > BR,
> > Jani.
> >
> >
> > --
> > Jani Nikula, Intel