On Tue, 10 May 2022, "Nautiyal, Ankit K" <ankit.k.nautiyal@xxxxxxxxx> wrote: > LGTM. > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx> Thanks for the review, pushed the lot already on Friday. BR, Jani. > > Regards, > > Ankit > > On 5/9/2022 5:33 PM, Jani Nikula wrote: >> Introduce new opaque type struct drm_edid to encapsulate the EDID data >> and the size allocated for it. The contents will be private to >> drm_edid.c. >> >> There are a number of reasons for adding a container around struct edid: >> >> * struct edid is a raw blob pointer to data that usually originates >> outside of the kernel. Its size is contained within the structure. >> >> * There's no way to attach meta information (such as allocated memory >> size) to struct edid. >> >> * Validation of the EDID blob and its size become crucial, and it's >> spread all over the subsystem, with varying levels of accuracy. >> >> * HDMI Forum has introduced an HF-EEODB extension that defines an >> override EDID size within an EDID extension. The size allocated for an >> EDID depends on whether the allocator understands the HF-EEODB >> extension. Given a struct edid *, it's impossible to know how much >> memory was actually allocated for it. >> >> There are also some reasons for making the container type struct >> drm_edid opaque and private to drm_edid.c: >> >> * Have only one place for creating and parsing the EDID, to avoid >> duplicating bugs. >> >> * Prepare for reading a pure DisplayID 2.0 from its own DDC address, and >> adding it within the same struct drm_edid container, transparently, >> and for all drivers. >> >> * With the idea that the drm_edid objects are immutable during their >> lifetimes, it will be possible to refcount them and reduce EDID >> copying everywhere (this is left for future work). >> >> Initially, just add the type. In follow-up, we'll start converting the >> guts of drm_edid.c to use it, and finally add interfaces around it. >> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/drm_edid.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index dcef92c8887a..480fd9fbe412 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -1567,6 +1567,15 @@ static const struct drm_display_mode edid_4k_modes[] = { >> >> /*** DDC fetch and block validation ***/ >> >> +/* >> + * The opaque EDID type, internal to drm_edid.c. >> + */ >> +struct drm_edid { >> + /* Size allocated for edid */ >> + size_t size; >> + const struct edid *edid; >> +}; >> + >> static int edid_extension_block_count(const struct edid *edid) >> { >> return edid->extensions; -- Jani Nikula, Intel Open Source Graphics Center