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

[Jammy Huang]

Hi Thomas,

Many thanks for your input. I will provide patch v3 in which zero-out the EDID buffers > 1.

Best regards,
Jammy

> 
> Hi Jammy
> 
> Am 18.08.26 um 08:43 schrieb 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.
> 
> I agree with Jani that the EDID should best be unmodified. But I've found that
> returning unprocessed EDID makes other components fail. In some cases, such
> as ast, at least the checksum needs to be corrected.
> 
> >
> > 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?
> 
> AFAIK there's currently no clear way to signal an EOF to the EDID
> parser.  Returning any errno code will make it fail hard.   IMHO that
> next best thing is to zero-out the EDID buffer, so that the parser stops
> silently. (Your v3, I think.) That's also easy enough to revert if/when
> the EDID parser gets a real EOF state.
> 
> Best regards
> Thomas
> 
> 
> >
> > 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
> 
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
> GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)
>