Message ID | 20240223223958.3887423-2-hsinyi@chromium.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:a81b:b0:108:e6aa:91d0 with SMTP id bq27csp880369dyb; Fri, 23 Feb 2024 14:41:05 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWjqyhTGtN4kKU/BlcuLaXEFH9oixlR066DRH9PNnsbAqafjJ37Nc9NNN07/KDrS7MTXqcHQCfxuLukDbUDMNEHvI2FJA== X-Google-Smtp-Source: AGHT+IHQnnJMgt+pqQ6gbZFFWweTabIuXo03ZEPYHZAv0OjRieEOvwM2vHh7TxJpys5T2hrl6vVo X-Received: by 2002:a05:6a20:c78f:b0:1a0:8c9c:acfa with SMTP id hk15-20020a056a20c78f00b001a08c9cacfamr1346505pzb.34.1708728065667; Fri, 23 Feb 2024 14:41:05 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708728065; cv=pass; d=google.com; s=arc-20160816; b=VQbKUNNDnFOZyC9aoj3Y12/kp9h79n7guTWQKNkOsHARHUzcnQm0nJNCB/BmRpbets qqQaMTEHP4U0BTWdG0kOIYTtuX61uXrqo+bLmk5L8hw59YMnw4VAPsz4UZ35nKi07q8Y TtzwNZ9T4bqX67+8Yv8vVaNjjKY5jYtNb0b5ZWXjkppK8eLI6a2p4EgSEqF8kYFSq2Fu 07zgN4CAMslct/lwB0OcD7lmJTwp7Y7fOh/6PUBP2swzmM80Ln/9RdvzXP3FTja+C0fh 7aWOCroPOuOmvMkqb4xOakHLnZI8sXERd9uwOr8erhaTuTWZ5+6bkTzfy4Q8slZaJELV r4aA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=j2NLOgJvFk9/xpCgoWWX0mwY7XC1SqoEPJetQl38NRg=; fh=aQYrFfOEQUVXz3UEt8TCHW0rQeO8SzGrahOjK1Zy04M=; b=jW5HABuwj7KQW3KFS4KwXZ3XCKJ7bPWy55kgZP7cEFTfzNsx1281XqwX/rCTXQs2ML xBnqty9muZDZ4V3GQciNHM/FIokzJWjgMCNTJv4sr4epewFAI8xd8zN4crVf4plvfAyH S5XlkOsWv74gWWkVlGPZlr10sygtt1YIJWCCRUryBNsWugiZi5GjV+3MOmB9Le1WbzHN KynmXNfFKfw8XImbL8wJxMsU58qeDYEeUwkPMN1u2gGnUfDcEKYHeRz9C0bBKrO/UvGs 7flgTmCIFLyVQqj/YAuShnK0SQvBt6F/A0KFQjSWc4RVK9k7lUmXnQbqDmWO8TTwB2zE psJA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=cpu4ndIt; arc=pass (i=1 spf=pass spfdomain=chromium.org dkim=pass dkdomain=chromium.org dmarc=pass fromdomain=chromium.org); spf=pass (google.com: domain of linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id x64-20020a636343000000b005d412d0f51esi12780669pgb.661.2024.02.23.14.41.05 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Feb 2024 14:41:05 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=cpu4ndIt; arc=pass (i=1 spf=pass spfdomain=chromium.org dkim=pass dkdomain=chromium.org dmarc=pass fromdomain=chromium.org); spf=pass (google.com: domain of linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-79275-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 72275288504 for <ouuuleilei@gmail.com>; Fri, 23 Feb 2024 22:40:41 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8A72514EFC9; Fri, 23 Feb 2024 22:40:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="cpu4ndIt" Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 65B9F143C63 for <linux-kernel@vger.kernel.org>; Fri, 23 Feb 2024 22:40:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708728007; cv=none; b=M6hWLNFmjp9uZGz8D+jF9gDpCFxy2YxOUG0H43rzKNs/MskD4sBmvlUUv7OrQAWVFgUx+ewyS5UcSuzo2zxPYBLUxAnVknsXz4wMr95vTxFtlwfkX0Eee1+jg0M0nj0/hS7AAxzWuI+LuzzAPbdbziu4r3bHEL39wSOreXs9q0M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708728007; c=relaxed/simple; bh=ZGM0H9OX6IuYQLFMOB8tArMAbsmHZckmY8VWfmMOH+0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=G3ZpgZSiHmoa1WwUTb+Su2vw1+0glKsCblDJn91cPpMMrUU0DU9Bg9SPhyWdXarjXVwD2yxx4tXrJl56TU+S/bLz+MSbX2ENEybRzq4e5YdGj2RbnpoNqIcc7D+y0WqfFOphLgw8yAZkYPEuEEDENBS24ArUJyAztqABnsHb9Ac= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=cpu4ndIt; arc=none smtp.client-ip=209.85.215.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Received: by mail-pg1-f171.google.com with SMTP id 41be03b00d2f7-5cedfc32250so1277353a12.0 for <linux-kernel@vger.kernel.org>; Fri, 23 Feb 2024 14:40:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1708728003; x=1709332803; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=j2NLOgJvFk9/xpCgoWWX0mwY7XC1SqoEPJetQl38NRg=; b=cpu4ndIt16D5/QrGZ6s67ePQzuC2xQc8W2t8TcoIXfeQpxWN+nVbhXIB4re+qdjKte thBi8YZdynxnmZ6eECAuP1F3WAjYlrFPCicTS9WFUyv7o3+IuEbqVqCYPquNp/bPwVO4 HqNRRLO9/iGBgOreXYOhJMYmqFKAF0S2vYAaw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708728003; x=1709332803; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=j2NLOgJvFk9/xpCgoWWX0mwY7XC1SqoEPJetQl38NRg=; b=FEQahFM731n67qDc0tV99lvj6dP+KErCWjvPDEH97NXdrvQL6tjXIb23fu/0DB8aWZ kKQX+1U3Uhjf9rGUKiQvtgTMZyM5aVkxItVXVoORRfCLp6YJTHbIGB4lJBx0QbK+ZfDF /TiB+aG3+yU5DhSYA+rJ6ggZINCELrYPN4NNEwMkJcLh2na7uYEENwg4JKLWVQti6go+ l746vw3dD/r94IeaVb7ruOvUPgdxzgujltumwhdPmgHWJb9NQFVl+D518Bas357RJePK v2BKBJEFFtQQg1S+F94gZHr6gVR/1/mRsvsoRsdLftrnGlZ/BbFL6mx+LdQBIrZVWlGI UUxQ== X-Forwarded-Encrypted: i=1; AJvYcCUA+kj9ysENdUqPkuZrEBkwvBoNW6KR7C2yQDevIeeQR77QdW4Va5z75mid52YTCSTRTB0QVtagf/a3gsa84QSr/QhEl2Z+beIEuMd8 X-Gm-Message-State: AOJu0Yy6Ramdjp/laUDXD1pKPKisQPoBWhjjQGEeZiPFyPutLhieHTdf AldKyrdEbRSN9/urcsulv/4+cUbQEGCe3MlESP0zlrYXWZkOj6pZkyzvIi2rUA== X-Received: by 2002:a17:90b:380d:b0:299:2da5:a843 with SMTP id mq13-20020a17090b380d00b002992da5a843mr1194908pjb.14.1708728003603; Fri, 23 Feb 2024 14:40:03 -0800 (PST) Received: from hsinyi.sjc.corp.google.com ([2620:15c:9d:2:8ff9:a089:c05c:9af]) by smtp.gmail.com with ESMTPSA id cz4-20020a17090ad44400b002966a13f2e9sm2032873pjb.37.2024.02.23.14.40.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Feb 2024 14:40:03 -0800 (PST) From: Hsin-Yi Wang <hsinyi@chromium.org> To: Douglas Anderson <dianders@chromium.org> Cc: Neil Armstrong <neil.armstrong@linaro.org>, Jessica Zhang <quic_jesszhan@quicinc.com>, Sam Ravnborg <sam@ravnborg.org>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] drm_edid: Add a function to get EDID base block Date: Fri, 23 Feb 2024 14:29:17 -0800 Message-ID: <20240223223958.3887423-2-hsinyi@chromium.org> X-Mailer: git-send-email 2.44.0.rc0.258.g7320e95886-goog In-Reply-To: <20240223223958.3887423-1-hsinyi@chromium.org> References: <20240223223958.3887423-1-hsinyi@chromium.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791731240050627633 X-GMAIL-MSGID: 1791731240050627633 |
Series |
Match panel hash for overridden mode
|
|
Commit Message
Hsin-Yi Wang
Feb. 23, 2024, 10:29 p.m. UTC
It's found that some panels have variants that they share the same panel id
although their EDID and names are different. Besides panel id, now we need
the hash of entire EDID base block to distinguish these panel variants.
Add drm_edid_get_base_block to returns the EDID base block, so caller can
further use it to get panel id and/or the hash.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/drm_edid.c | 55 +++++++++++++++++--------------
drivers/gpu/drm/panel/panel-edp.c | 8 +++--
include/drm/drm_edid.h | 3 +-
3 files changed, 38 insertions(+), 28 deletions(-)
Comments
Hi, On Fri, Feb 23, 2024 at 2:40 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > @@ -2770,58 +2770,63 @@ 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 base block > + * @base_bock: EDID base block that contains panel ID. s/base_bock/base_block, as identified by: scripts/kernel-doc -v drivers/gpu/drm/drm_edid.c | less 2>&1 drivers/gpu/drm/drm_edid.c:2787: warning: Function parameter or struct member 'base_block' not described in 'drm_edid_get_panel_id' drivers/gpu/drm/drm_edid.c:2787: warning: Excess function parameter 'base_bock' description in 'drm_edid_get_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(void *base_block) > +{ > + return edid_extract_panel_id(base_block); > +} > +EXPORT_SYMBOL(drm_edid_get_panel_id); > + > +/** > + * drm_edid_get_base_block - Get a panel's EDID base block > + * @adapter: I2C adapter to use for DDC > + * > + * This function returns the first block of the EDID of a panel. > + * > * This function is intended to be used during early probing on devices where > * more than one panel might be present. Because of its intended use it must > - * assume that the EDID of the panel is correct, at least as far as the ID > - * is concerned (in other words, we don't process any overrides here). > + * assume that the EDID of the panel is correct, at least as far as the base > + * block is concerned (in other words, we don't process any overrides here). > * > * NOTE: it's expected that this function and drm_do_get_edid() will both > * be read the EDID, but there is no caching between them. Since we're only > * reading the first block, hopefully this extra overhead won't be too big. > * > - * 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. > + * Caller should free the base block after use. Don't you need a "Return:" clause here to document what you're returning? Other than the kernel-doc nits, this looks fine to me. Reviewed-by: Douglas Anderson <dianders@chromium.org> It'll probably need at least an Ack from someone else in the DRM community before it can land, though, since this is touching a core file. -Doug
On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > It's found that some panels have variants that they share the same panel id > although their EDID and names are different. Besides panel id, now we need > the hash of entire EDID base block to distinguish these panel variants. > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > further use it to get panel id and/or the hash. Please reconsider the whole approach here. Please let's not add single-use special case functions to read an EDID base block. Please consider reading the whole EDID, using the regular EDID reading functions, and use that instead. Most likely you'll only have 1-2 blocks anyway. And you might consider caching the EDID in struct panel_edp if reading the entire EDID is too slow. (And if it is, this is probably sensible even if the EDID only consists of one block.) Anyway, please do *not* merge this as-is. BR, Jani. > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > --- > drivers/gpu/drm/drm_edid.c | 55 +++++++++++++++++-------------- > drivers/gpu/drm/panel/panel-edp.c | 8 +++-- > include/drm/drm_edid.h | 3 +- > 3 files changed, 38 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 923c4423151c..55598ca4a5d1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2770,58 +2770,63 @@ 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 base block > + * @base_bock: EDID base block 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(void *base_block) > +{ > + return edid_extract_panel_id(base_block); > +} > +EXPORT_SYMBOL(drm_edid_get_panel_id); > + > +/** > + * drm_edid_get_base_block - Get a panel's EDID base block > + * @adapter: I2C adapter to use for DDC > + * > + * This function returns the first block of the EDID of a panel. > + * > * This function is intended to be used during early probing on devices where > * more than one panel might be present. Because of its intended use it must > - * assume that the EDID of the panel is correct, at least as far as the ID > - * is concerned (in other words, we don't process any overrides here). > + * assume that the EDID of the panel is correct, at least as far as the base > + * block is concerned (in other words, we don't process any overrides here). > * > * NOTE: it's expected that this function and drm_do_get_edid() will both > * be read the EDID, but there is no caching between them. Since we're only > * reading the first block, hopefully this extra overhead won't be too big. > * > - * 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. > + * Caller should free the base block after use. > */ > - > -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter) > +void *drm_edid_get_base_block(struct i2c_adapter *adapter) > { > enum edid_block_status status; > void *base_block; > - u32 panel_id = 0; > - > - /* > - * There are no manufacturer IDs of 0, so if there is a problem reading > - * the EDID then we'll just return 0. > - */ > > base_block = kzalloc(EDID_LENGTH, GFP_KERNEL); > if (!base_block) > - return 0; > + return NULL; > > status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter); > > edid_block_status_print(status, base_block, 0); > > - if (edid_block_status_valid(status, edid_block_tag(base_block))) > - panel_id = edid_extract_panel_id(base_block); > - else > + if (!edid_block_status_valid(status, edid_block_tag(base_block))) { > edid_block_dump(KERN_NOTICE, base_block, 0); > + return NULL; > + } > > - kfree(base_block); > - > - return panel_id; > + return base_block; > } > -EXPORT_SYMBOL(drm_edid_get_panel_id); > +EXPORT_SYMBOL(drm_edid_get_base_block); > > /** > * drm_get_edid_switcheroo - get EDID data for a vga_switcheroo output > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index bd71d239272a..f6ddbaa103b5 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -763,6 +763,7 @@ static const struct edp_panel_entry *find_edp_panel(u32 panel_id); > static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) > { > struct panel_desc *desc; > + void *base_block; > u32 panel_id; > char vend[4]; > u16 product_id; > @@ -792,8 +793,11 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) > goto exit; > } > > - panel_id = drm_edid_get_panel_id(panel->ddc); > - if (!panel_id) { > + base_block = drm_edid_get_base_block(panel->ddc); > + if (base_block) { > + panel_id = drm_edid_get_panel_id(base_block); > + kfree(base_block); > + } else { > dev_err(dev, "Couldn't identify panel via EDID\n"); > ret = -EIO; > goto exit; > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > index 7923bc00dc7a..56b60f9204d3 100644 > --- a/include/drm/drm_edid.h > +++ b/include/drm/drm_edid.h > @@ -410,7 +410,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > void *data); > struct edid *drm_get_edid(struct drm_connector *connector, > struct i2c_adapter *adapter); > -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter); > +void *drm_edid_get_base_block(struct i2c_adapter *adapter); > +u32 drm_edid_get_panel_id(void *base_block); > struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, > struct i2c_adapter *adapter); > struct edid *drm_edid_duplicate(const struct edid *edid);
Hi, On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > It's found that some panels have variants that they share the same panel id > > although their EDID and names are different. Besides panel id, now we need > > the hash of entire EDID base block to distinguish these panel variants. > > > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > > further use it to get panel id and/or the hash. > > Please reconsider the whole approach here. > > Please let's not add single-use special case functions to read an EDID > base block. > > Please consider reading the whole EDID, using the regular EDID reading > functions, and use that instead. > > Most likely you'll only have 1-2 blocks anyway. And you might consider > caching the EDID in struct panel_edp if reading the entire EDID is too > slow. (And if it is, this is probably sensible even if the EDID only > consists of one block.) That makes a lot of sense! Not quite sure why I didn't just read the whole EDID in the first place when trying to get the panel ID. -Doug
On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > It's found that some panels have variants that they share the same panel id > > although their EDID and names are different. Besides panel id, now we need > > the hash of entire EDID base block to distinguish these panel variants. > > > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > > further use it to get panel id and/or the hash. > > Please reconsider the whole approach here. > > Please let's not add single-use special case functions to read an EDID > base block. > > Please consider reading the whole EDID, using the regular EDID reading > functions, and use that instead. > > Most likely you'll only have 1-2 blocks anyway. And you might consider > caching the EDID in struct panel_edp if reading the entire EDID is too > slow. (And if it is, this is probably sensible even if the EDID only > consists of one block.) > > Anyway, please do *not* merge this as-is. > hi Jani, I sent a v2 here implementing this method: https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/ We still have to read edid twice due to: 1. The first caller is in panel probe, at that time, connector is still unknown, so we can't update connector status (eg. update edid_corrupt). 2. It's possible that the connector can have some override (drm_edid_override_get) to EDID, that is still unknown during the first read. > BR, > Jani. > > > > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > > --- > > drivers/gpu/drm/drm_edid.c | 55 +++++++++++++++++-------------- > > drivers/gpu/drm/panel/panel-edp.c | 8 +++-- > > include/drm/drm_edid.h | 3 +- > > 3 files changed, 38 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 923c4423151c..55598ca4a5d1 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -2770,58 +2770,63 @@ 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 base block > > + * @base_bock: EDID base block 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(void *base_block) > > +{ > > + return edid_extract_panel_id(base_block); > > +} > > +EXPORT_SYMBOL(drm_edid_get_panel_id); > > + > > +/** > > + * drm_edid_get_base_block - Get a panel's EDID base block > > + * @adapter: I2C adapter to use for DDC > > + * > > + * This function returns the first block of the EDID of a panel. > > + * > > * This function is intended to be used during early probing on devices where > > * more than one panel might be present. Because of its intended use it must > > - * assume that the EDID of the panel is correct, at least as far as the ID > > - * is concerned (in other words, we don't process any overrides here). > > + * assume that the EDID of the panel is correct, at least as far as the base > > + * block is concerned (in other words, we don't process any overrides here). > > * > > * NOTE: it's expected that this function and drm_do_get_edid() will both > > * be read the EDID, but there is no caching between them. Since we're only > > * reading the first block, hopefully this extra overhead won't be too big. > > * > > - * 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. > > + * Caller should free the base block after use. > > */ > > - > > -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter) > > +void *drm_edid_get_base_block(struct i2c_adapter *adapter) > > { > > enum edid_block_status status; > > void *base_block; > > - u32 panel_id = 0; > > - > > - /* > > - * There are no manufacturer IDs of 0, so if there is a problem reading > > - * the EDID then we'll just return 0. > > - */ > > > > base_block = kzalloc(EDID_LENGTH, GFP_KERNEL); > > if (!base_block) > > - return 0; > > + return NULL; > > > > status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter); > > > > edid_block_status_print(status, base_block, 0); > > > > - if (edid_block_status_valid(status, edid_block_tag(base_block))) > > - panel_id = edid_extract_panel_id(base_block); > > - else > > + if (!edid_block_status_valid(status, edid_block_tag(base_block))) { > > edid_block_dump(KERN_NOTICE, base_block, 0); > > + return NULL; > > + } > > > > - kfree(base_block); > > - > > - return panel_id; > > + return base_block; > > } > > -EXPORT_SYMBOL(drm_edid_get_panel_id); > > +EXPORT_SYMBOL(drm_edid_get_base_block); > > > > /** > > * drm_get_edid_switcheroo - get EDID data for a vga_switcheroo output > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > > index bd71d239272a..f6ddbaa103b5 100644 > > --- a/drivers/gpu/drm/panel/panel-edp.c > > +++ b/drivers/gpu/drm/panel/panel-edp.c > > @@ -763,6 +763,7 @@ static const struct edp_panel_entry *find_edp_panel(u32 panel_id); > > static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) > > { > > struct panel_desc *desc; > > + void *base_block; > > u32 panel_id; > > char vend[4]; > > u16 product_id; > > @@ -792,8 +793,11 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) > > goto exit; > > } > > > > - panel_id = drm_edid_get_panel_id(panel->ddc); > > - if (!panel_id) { > > + base_block = drm_edid_get_base_block(panel->ddc); > > + if (base_block) { > > + panel_id = drm_edid_get_panel_id(base_block); > > + kfree(base_block); > > + } else { > > dev_err(dev, "Couldn't identify panel via EDID\n"); > > ret = -EIO; > > goto exit; > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > > index 7923bc00dc7a..56b60f9204d3 100644 > > --- a/include/drm/drm_edid.h > > +++ b/include/drm/drm_edid.h > > @@ -410,7 +410,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > > void *data); > > struct edid *drm_get_edid(struct drm_connector *connector, > > struct i2c_adapter *adapter); > > -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter); > > +void *drm_edid_get_base_block(struct i2c_adapter *adapter); > > +u32 drm_edid_get_panel_id(void *base_block); > > struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, > > struct i2c_adapter *adapter); > > struct edid *drm_edid_duplicate(const struct edid *edid); > > -- > Jani Nikula, Intel
Hi, On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > > It's found that some panels have variants that they share the same panel id > > > although their EDID and names are different. Besides panel id, now we need > > > the hash of entire EDID base block to distinguish these panel variants. > > > > > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > > > further use it to get panel id and/or the hash. > > > > Please reconsider the whole approach here. > > > > Please let's not add single-use special case functions to read an EDID > > base block. > > > > Please consider reading the whole EDID, using the regular EDID reading > > functions, and use that instead. > > > > Most likely you'll only have 1-2 blocks anyway. And you might consider > > caching the EDID in struct panel_edp if reading the entire EDID is too > > slow. (And if it is, this is probably sensible even if the EDID only > > consists of one block.) > > > > Anyway, please do *not* merge this as-is. > > > > hi Jani, > > I sent a v2 here implementing this method: > https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/ > > We still have to read edid twice due to: > 1. The first caller is in panel probe, at that time, connector is > still unknown, so we can't update connector status (eg. update > edid_corrupt). > 2. It's possible that the connector can have some override > (drm_edid_override_get) to EDID, that is still unknown during the > first read. I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the fact that we can't cache the EDID (because we don't yet have a "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that allows reading just the first block. If we try to boot a device with a multi-block EDID we're now wastefully reading all the blocks of the EDID twice at bootup which will slow boot time. If you can see a good solution to avoid reading the EDID twice then that would be amazing, but if not it seems like we should go back to what's here in v1. What do you think? Anyone else have any opinions? -Doug
On Wed, 28 Feb 2024, Doug Anderson <dianders@chromium.org> wrote: > Hi, > > On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: >> >> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: >> > >> > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: >> > > It's found that some panels have variants that they share the same panel id >> > > although their EDID and names are different. Besides panel id, now we need >> > > the hash of entire EDID base block to distinguish these panel variants. >> > > >> > > Add drm_edid_get_base_block to returns the EDID base block, so caller can >> > > further use it to get panel id and/or the hash. >> > >> > Please reconsider the whole approach here. >> > >> > Please let's not add single-use special case functions to read an EDID >> > base block. >> > >> > Please consider reading the whole EDID, using the regular EDID reading >> > functions, and use that instead. >> > >> > Most likely you'll only have 1-2 blocks anyway. And you might consider >> > caching the EDID in struct panel_edp if reading the entire EDID is too >> > slow. (And if it is, this is probably sensible even if the EDID only >> > consists of one block.) >> > >> > Anyway, please do *not* merge this as-is. >> > >> >> hi Jani, >> >> I sent a v2 here implementing this method: >> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/ >> >> We still have to read edid twice due to: >> 1. The first caller is in panel probe, at that time, connector is >> still unknown, so we can't update connector status (eg. update >> edid_corrupt). >> 2. It's possible that the connector can have some override >> (drm_edid_override_get) to EDID, that is still unknown during the >> first read. > > I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the > fact that we can't cache the EDID (because we don't yet have a > "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that > allows reading just the first block. If we try to boot a device with a > multi-block EDID we're now wastefully reading all the blocks of the > EDID twice at bootup which will slow boot time. > > If you can see a good solution to avoid reading the EDID twice then > that would be amazing, but if not it seems like we should go back to > what's here in v1. What do you think? Anyone else have any opinions? I haven't replied so far, because I've been going back and forth with this. I'm afraid I don't really like either approach now. Handling the no connector case in v2 is a bit ugly too. :( Seems like you only need this to extend the panel ID with a hash. And panel-edp.c is the only user of drm_edid_get_panel_id(). And EDID quirks in drm_edid.c could theoretically hit the same problem you're solving. So maybe something like: u32 drm_edid_get_panel_id(struct i2c_adapter *adapter, u32 *hash); or if you want to be fancy add a struct capturing both id and hash: bool drm_edid_get_panel_id(struct i2c_adapter *adapter, struct drm_edid_panel_id *id); And put the hash (or whatever mechanism you have) computation in drm_edid.c. Just hide it all in drm_edid.c, and keep the EDID interfaces neat. How would that work for you? BR, Jani.
Hi, On Thu, Feb 29, 2024 at 8:43 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Wed, 28 Feb 2024, Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > > > On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > >> > >> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linuxintel.com> wrote: > >> > > >> > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > >> > > It's found that some panels have variants that they share the same panel id > >> > > although their EDID and names are different. Besides panel id, now we need > >> > > the hash of entire EDID base block to distinguish these panel variants. > >> > > > >> > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > >> > > further use it to get panel id and/or the hash. > >> > > >> > Please reconsider the whole approach here. > >> > > >> > Please let's not add single-use special case functions to read an EDID > >> > base block. > >> > > >> > Please consider reading the whole EDID, using the regular EDID reading > >> > functions, and use that instead. > >> > > >> > Most likely you'll only have 1-2 blocks anyway. And you might consider > >> > caching the EDID in struct panel_edp if reading the entire EDID is too > >> > slow. (And if it is, this is probably sensible even if the EDID only > >> > consists of one block.) > >> > > >> > Anyway, please do *not* merge this as-is. > >> > > >> > >> hi Jani, > >> > >> I sent a v2 here implementing this method: > >> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/ > >> > >> We still have to read edid twice due to: > >> 1. The first caller is in panel probe, at that time, connector is > >> still unknown, so we can't update connector status (eg. update > >> edid_corrupt). > >> 2. It's possible that the connector can have some override > >> (drm_edid_override_get) to EDID, that is still unknown during the > >> first read. > > > > I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the > > fact that we can't cache the EDID (because we don't yet have a > > "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that > > allows reading just the first block. If we try to boot a device with a > > multi-block EDID we're now wastefully reading all the blocks of the > > EDID twice at bootup which will slow boot time. > > > > If you can see a good solution to avoid reading the EDID twice then > > that would be amazing, but if not it seems like we should go back to > > what's here in v1. What do you think? Anyone else have any opinions? > > I haven't replied so far, because I've been going back and forth with > this. I'm afraid I don't really like either approach now. Handling the > no connector case in v2 is a bit ugly too. :( > > Seems like you only need this to extend the panel ID with a hash. And > panel-edp.c is the only user of drm_edid_get_panel_id(). And EDID quirks > in drm_edid.c could theoretically hit the same problem you're solving. Good point. That would imply that more of the logic could go into "drm_edid.c" in case the EDID quirks code eventually needs it. > So maybe something like: > > u32 drm_edid_get_panel_id(struct i2c_adapter *adapter, u32 *hash); > > or if you want to be fancy add a struct capturing both id and hash: > > bool drm_edid_get_panel_id(struct i2c_adapter *adapter, struct drm_edid_panel_id *id); > > And put the hash (or whatever mechanism you have) computation in > drm_edid.c. Just hide it all in drm_edid.c, and keep the EDID interfaces > neat. > > How would that work for you? The problem is that Dmitry didn't like the idea of using a hash and in v2 Hsin-Yi has moved to using the name of the display. ...except of course that eDP panels don't always properly specify "EDID_DETAIL_MONITOR_NAME". See the discussion [1]. If you want to see some of the EDIDs involved, you can see Hsin-Yi's post [2]. The panels included stuff like this: Alphanumeric Data String: 'AUO' Alphanumeric Data String: 'B116XAN04.0 ' The fact that there is more than one string in there makes it hard to just "return" the display name in a generic way. The way Hsin-Yi's code was doing it was that it would consider it a match if the panel name was in any of the strings... How about this as a solution: we change drm_edid_get_panel_id() to return an opaque type (struct drm_edid_panel_id_blob) that's really just the first block of the EDID but we can all pretend that it isn't. Then we can add a function in drm_edid.c that takes this opaque blob, a 32-bit integer (as per drm_edid_encode_panel_id()), and a string name and it can tell us if the blob matches? [1] https://lore.kernel.org/r/CAD=FV=VMVr+eJ7eyuLGa671fMgH6ZX9zPOkbKzYJ0H79MZ2k9A@mail.gmail.com [2] https://lore.kernel.org/r/CAJMQK-gfKbdPhYJeCJ5UX0dNrx3y-EmLsTiv9nj+U3Rmej38pw@mail.gmail.com
On Thu, 29 Feb 2024 at 19:11, Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Feb 29, 2024 at 8:43 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > > On Wed, 28 Feb 2024, Doug Anderson <dianders@chromium.org> wrote: > > > Hi, > > > > > > On Tue, Feb 27, 2024 at 5:27 PM Hsin-Yi Wang <hsinyi@chromiumorg> wrote: > > >> > > >> On Tue, Feb 27, 2024 at 1:09 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > >> > > > >> > On Fri, 23 Feb 2024, Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > >> > > It's found that some panels have variants that they share the same panel id > > >> > > although their EDID and names are different. Besides panel id, now we need > > >> > > the hash of entire EDID base block to distinguish these panel variants. > > >> > > > > >> > > Add drm_edid_get_base_block to returns the EDID base block, so caller can > > >> > > further use it to get panel id and/or the hash. > > >> > > > >> > Please reconsider the whole approach here. > > >> > > > >> > Please let's not add single-use special case functions to read an EDID > > >> > base block. > > >> > > > >> > Please consider reading the whole EDID, using the regular EDID reading > > >> > functions, and use that instead. > > >> > > > >> > Most likely you'll only have 1-2 blocks anyway. And you might consider > > >> > caching the EDID in struct panel_edp if reading the entire EDID is too > > >> > slow. (And if it is, this is probably sensible even if the EDID only > > >> > consists of one block.) > > >> > > > >> > Anyway, please do *not* merge this as-is. > > >> > > > >> > > >> hi Jani, > > >> > > >> I sent a v2 here implementing this method: > > >> https://lore.kernel.org/lkml/20240228011133.1238439-2-hsinyi@chromium.org/ > > >> > > >> We still have to read edid twice due to: > > >> 1. The first caller is in panel probe, at that time, connector is > > >> still unknown, so we can't update connector status (eg. update > > >> edid_corrupt). > > >> 2. It's possible that the connector can have some override > > >> (drm_edid_override_get) to EDID, that is still unknown during the > > >> first read. > > > > > > I'll also comment in Hsin-Yi's v2, but given Hsin-Yi's digging and the > > > fact that we can't cache the EDID (because we don't yet have a > > > "drm_connector"), I'd much prefer Hsin-Yi's solution here from v1 that > > > allows reading just the first block. If we try to boot a device with a > > > multi-block EDID we're now wastefully reading all the blocks of the > > > EDID twice at bootup which will slow boot time. > > > > > > If you can see a good solution to avoid reading the EDID twice then > > > that would be amazing, but if not it seems like we should go back to > > > what's here in v1. What do you think? Anyone else have any opinions? > > > > I haven't replied so far, because I've been going back and forth with > > this. I'm afraid I don't really like either approach now. Handling the > > no connector case in v2 is a bit ugly too. :( > > > > Seems like you only need this to extend the panel ID with a hash. And > > panel-edp.c is the only user of drm_edid_get_panel_id(). And EDID quirks > > in drm_edid.c could theoretically hit the same problem you're solving. > > Good point. That would imply that more of the logic could go into > "drm_edid.c" in case the EDID quirks code eventually needs it. > > > > So maybe something like: > > > > u32 drm_edid_get_panel_id(struct i2c_adapter *adapter, u32 *hash); > > > > or if you want to be fancy add a struct capturing both id and hash: > > > > bool drm_edid_get_panel_id(struct i2c_adapter *adapter, struct drm_edid_panel_id *id); > > > > And put the hash (or whatever mechanism you have) computation in > > drm_edid.c. Just hide it all in drm_edid.c, and keep the EDID interfaces > > neat. > > > > How would that work for you? > > The problem is that Dmitry didn't like the idea of using a hash and in > v2 Hsin-Yi has moved to using the name of the display. ...except of > course that eDP panels don't always properly specify > "EDID_DETAIL_MONITOR_NAME". See the discussion [1]. If you want to see > some of the EDIDs involved, you can see Hsin-Yi's post [2]. The panels > included stuff like this: > > Alphanumeric Data String: 'AUO' > Alphanumeric Data String: 'B116XAN04.0 ' > > The fact that there is more than one string in there makes it hard to > just "return" the display name in a generic way. The way Hsin-Yi's > code was doing it was that it would consider it a match if the panel > name was in any of the strings... > > How about this as a solution: we change drm_edid_get_panel_id() to > return an opaque type (struct drm_edid_panel_id_blob) that's really > just the first block of the EDID but we can all pretend that it isn't. > Then we can add a function in drm_edid.c that takes this opaque blob, > a 32-bit integer (as per drm_edid_encode_panel_id()), and a string > name and it can tell us if the blob matches? Would it be easier to push drm_edid_match to drm_edid.c? It looks way more simpler than the opaque blob. > [1] https://lore.kernel.org/r/CAD=FV=VMVr+eJ7eyuLGa671fMgH6ZX9zPOkbKzYJ0H79MZ2k9A@mail.gmail.com > [2] https://lore.kernel.org/r/CAJMQK-gfKbdPhYJeCJ5UX0dNrx3y-EmLsTiv9nj+U3Rmej38pw@mail.gmail.com
Hi, On Sun, Mar 3, 2024 at 1:30 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > The problem is that Dmitry didn't like the idea of using a hash and in > > v2 Hsin-Yi has moved to using the name of the display. ...except of > > course that eDP panels don't always properly specify > > "EDID_DETAIL_MONITOR_NAME". See the discussion [1]. If you want to see > > some of the EDIDs involved, you can see Hsin-Yi's post [2]. The panels > > included stuff like this: > > > > Alphanumeric Data String: 'AUO' > > Alphanumeric Data String: 'B116XAN04.0 ' > > > > The fact that there is more than one string in there makes it hard to > > just "return" the display name in a generic way. The way Hsin-Yi's > > code was doing it was that it would consider it a match if the panel > > name was in any of the strings... > > > > How about this as a solution: we change drm_edid_get_panel_id() to > > return an opaque type (struct drm_edid_panel_id_blob) that's really > > just the first block of the EDID but we can all pretend that it isn't. > > Then we can add a function in drm_edid.c that takes this opaque blob, > > a 32-bit integer (as per drm_edid_encode_panel_id()), and a string > > name and it can tell us if the blob matches? > > Would it be easier to push drm_edid_match to drm_edid.c? It looks way > more simpler than the opaque blob. Yeah, that sounds reasonable / cleaner to me. Good idea! Maybe Hsin-Yi will be able to try this out and see if there's a reason it wouldn't work. -Doug
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 923c4423151c..55598ca4a5d1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2770,58 +2770,63 @@ 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 base block + * @base_bock: EDID base block 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(void *base_block) +{ + return edid_extract_panel_id(base_block); +} +EXPORT_SYMBOL(drm_edid_get_panel_id); + +/** + * drm_edid_get_base_block - Get a panel's EDID base block + * @adapter: I2C adapter to use for DDC + * + * This function returns the first block of the EDID of a panel. + * * This function is intended to be used during early probing on devices where * more than one panel might be present. Because of its intended use it must - * assume that the EDID of the panel is correct, at least as far as the ID - * is concerned (in other words, we don't process any overrides here). + * assume that the EDID of the panel is correct, at least as far as the base + * block is concerned (in other words, we don't process any overrides here). * * NOTE: it's expected that this function and drm_do_get_edid() will both * be read the EDID, but there is no caching between them. Since we're only * reading the first block, hopefully this extra overhead won't be too big. * - * 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. + * Caller should free the base block after use. */ - -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter) +void *drm_edid_get_base_block(struct i2c_adapter *adapter) { enum edid_block_status status; void *base_block; - u32 panel_id = 0; - - /* - * There are no manufacturer IDs of 0, so if there is a problem reading - * the EDID then we'll just return 0. - */ base_block = kzalloc(EDID_LENGTH, GFP_KERNEL); if (!base_block) - return 0; + return NULL; status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter); edid_block_status_print(status, base_block, 0); - if (edid_block_status_valid(status, edid_block_tag(base_block))) - panel_id = edid_extract_panel_id(base_block); - else + if (!edid_block_status_valid(status, edid_block_tag(base_block))) { edid_block_dump(KERN_NOTICE, base_block, 0); + return NULL; + } - kfree(base_block); - - return panel_id; + return base_block; } -EXPORT_SYMBOL(drm_edid_get_panel_id); +EXPORT_SYMBOL(drm_edid_get_base_block); /** * drm_get_edid_switcheroo - get EDID data for a vga_switcheroo output diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index bd71d239272a..f6ddbaa103b5 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -763,6 +763,7 @@ static const struct edp_panel_entry *find_edp_panel(u32 panel_id); static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) { struct panel_desc *desc; + void *base_block; u32 panel_id; char vend[4]; u16 product_id; @@ -792,8 +793,11 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel) goto exit; } - panel_id = drm_edid_get_panel_id(panel->ddc); - if (!panel_id) { + base_block = drm_edid_get_base_block(panel->ddc); + if (base_block) { + panel_id = drm_edid_get_panel_id(base_block); + kfree(base_block); + } else { dev_err(dev, "Couldn't identify panel via EDID\n"); ret = -EIO; goto exit; diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 7923bc00dc7a..56b60f9204d3 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -410,7 +410,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, void *data); struct edid *drm_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter); -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter); +void *drm_edid_get_base_block(struct i2c_adapter *adapter); +u32 drm_edid_get_panel_id(void *base_block); struct edid *drm_get_edid_switcheroo(struct drm_connector *connector, struct i2c_adapter *adapter); struct edid *drm_edid_duplicate(const struct edid *edid);