[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87wmqew6ie.fsf@intel.com>
Date: Thu, 07 Mar 2024 14:50:17 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Doug Anderson <dianders@...omium.org>, Hsin-Yi Wang <hsinyi@...omium.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Neil Armstrong
<neil.armstrong@...aro.org>, Jessica Zhang <quic_jesszhan@...cinc.com>,
Sam Ravnborg <sam@...nborg.org>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/6] drm_edid: Add a function to get EDID base block
On Wed, 06 Mar 2024, Doug Anderson <dianders@...omium.org> wrote:
> Hi,
>
> On Wed, Mar 6, 2024 at 12:04 PM Hsin-Yi Wang <hsinyi@...omium.org> wrote:
>>
>> @@ -2764,58 +2764,71 @@ static u32 edid_extract_panel_id(const struct edid *edid)
>> }
>>
>> /**
>> - * drm_edid_get_panel_id - Get a panel's ID through DDC
>> - * @adapter: I2C adapter to use for DDC
>> + * drm_edid_get_panel_id - Get a panel's ID from EDID
>> + * @drm_edid: EDID that contains panel ID.
>> *
>> - * This function reads the first block of the EDID of a panel and (assuming
>> + * This function uses the first block of the EDID of a panel and (assuming
>> * that the EDID is valid) extracts the ID out of it. The ID is a 32-bit value
>> * (16 bits of manufacturer ID and 16 bits of per-manufacturer ID) that's
>> * supposed to be different for each different modem of panel.
>> *
>> + * Return: A 32-bit ID that should be different for each make/model of panel.
>> + * See the functions drm_edid_encode_panel_id() and
>> + * drm_edid_decode_panel_id() for some details on the structure of this
>> + * ID.
>> + */
>> +u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid)
>> +{
>
> I'd leave it up to Jani, but I'd wonder whether we need to confirm
> drm_edid->size here is at least as big as the base block. In other
> words: is there ever any chance that someone would have allocated a
> struct drm_edid but not actually read a full base block into it?
On the one hand, I've tried to make all the drm_edid based interfaces
handle all the cases (drm_edid == NULL, drm_edid->edid == NULL,
drm_edid->size < required) gracefully, but on the other hand, panel-edp
is the only user and this would go boom for you quickly if you passed in
a bogus drm_edid.
Adding the checks is definitely not wrong, but I'm not insisting.
Reviewed-by: Jani Nikula <jani.nikula@...el.com>
>
> In any case:
>
> Reviewed-by: Douglas Anderson <dianders@...omium.org>
--
Jani Nikula, Intel
Powered by blists - more mailing lists