Hi,
On Tue, Feb 27, 2024 at 5:11 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> Some panels are interested in the EDID during early probe when connector
> is still unknown.
>
> Add a function drm_get_edid_no_connector() to get edid without connector.
> No functional change for existing usage.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> ---
> v1->v2:
> add a function to return the entire edid without updating connector.
> ---
> drivers/gpu/drm/drm_edid.c | 45 ++++++++++++++++++++++++++++----------
> include/drm/drm_edid.h | 1 +
> 2 files changed, 34 insertions(+), 12 deletions(-)
I'll respond in the discussion in v1 too, but overall I'm not a fan of
reading the whole EDID twice at bootup. Personally I'd love to see us
to back to just reading the base block like in v1, but I guess we can
see what Jani and others say.
> @@ -2385,18 +2385,20 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
> if (status == EDID_BLOCK_READ_FAIL)
> goto fail;
>
> - /* FIXME: Clarify what a corrupt EDID actually means. */
> - if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
> - connector->edid_corrupt = false;
> - else
> - connector->edid_corrupt = true;
> + if (connector) {
> + /* FIXME: Clarify what a corrupt EDID actually means. */
> + if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
> + connector->edid_corrupt = false;
> + else
> + connector->edid_corrupt = true;
>
> - if (!edid_block_status_valid(status, edid_block_tag(edid))) {
> - if (status == EDID_BLOCK_ZERO)
> - connector->null_edid_counter++;
> + if (!edid_block_status_valid(status, edid_block_tag(edid))) {
> + if (status == EDID_BLOCK_ZERO)
> + connector->null_edid_counter++;
>
> - connector_bad_edid(connector, edid, 1);
> - goto fail;
> + connector_bad_edid(connector, edid, 1);
> + goto fail;
This "goto fail" is now only run "if (connector)" which means that
you're not properly checking if the EDID is valid when "connector ==
NULL", right? That seems like a bug unless I missed something...
