On Tue, 05 Mar 2024, Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> 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
> more information from the EDID base block to distinguish these panel
> variants.
>
> Add drm_edid_read_base_block() to return the EDID base block, which is
> wrapped in struct drm_edid.
>
> Caller can further use it to get panel id or check if the block contains
> certain strings, such as panel name.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> ---
> v3->v4: change drm_edid_read_base_block return type to drm_edid.
> ---
> drivers/gpu/drm/drm_edid.c | 63 +++++++++++++++++++------------
> drivers/gpu/drm/panel/panel-edp.c | 8 +++-
> include/drm/drm_edid.h | 3 +-
> 3 files changed, 46 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 923c4423151c..f9e09f327f81 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2770,58 +2770,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)
> +{
> + return edid_extract_panel_id(drm_edid->edid);
> +}
> +EXPORT_SYMBOL(drm_edid_get_panel_id);
> +
> +/**
> + * drm_edid_read_base_block - Get a panel's EDID base block
> + * @adapter: I2C adapter to use for DDC
> + *
> + * This function returns the drm_edid containing 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).
> + *
> + * Caller should call drm_edid_free() after use.
> *
> * 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.
> + * WARNING: Only use this function when the connector is unknown. For example,
> + * during the early probe of panel. The EDID read from the function is temporary
> + * and should be replaced by the full EDID returned from other drm_edid_read.
> + *
> + * Return: Pointer to allocated EDID base block, or NULL on any failure.
> */
> -
> -u32 drm_edid_get_panel_id(struct i2c_adapter *adapter)
> +const struct drm_edid *drm_edid_read_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);
> + kfree(base_block);
> + return NULL;
> + }
>
> - kfree(base_block);
> -
> - return panel_id;
> + return drm_edid_alloc(base_block, EDID_LENGTH);
This leaks base_block. Please use _drm_edid_alloc() (with underscore) to
only allocate the container without kmemduping the data.
Otherwise LGTM.
BR,
Jani.
> }
> -EXPORT_SYMBOL(drm_edid_get_panel_id);
> +EXPORT_SYMBOL(drm_edid_read_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 745f3e48f02a..d094cfc43da8 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -766,6 +766,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;
> + const struct drm_edid *base_block;
> u32 panel_id;
> char vend[4];
> u16 product_id;
> @@ -795,8 +796,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_read_base_block(panel->ddc);
> + if (base_block) {
> + panel_id = drm_edid_get_panel_id(base_block);
> + drm_edid_free(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..9686a7cee6a6 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);
> +const struct drm_edid *drm_edid_read_base_block(struct i2c_adapter *adapter);
> +u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
> 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
