Re: [PATCH 2/2] drm/panel: panel-edp: Match with panel hash for overridden modes

From: Dmitry Baryshkov
Date: Mon Feb 26 2024 - 21:28:48 EST

On Sat, 24 Feb 2024 at 00:40, 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. One of the variants requires
> using overridden modes to resolve glitching issue as described in commit
> 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode"). Other variants should
> use the modes parsed from EDID.
>
> For example, AUO 0x405c B116XAK01.0, it has at least 2 different variants
> that EDID and panel name are different, but using the same panel id. One of
> the variants require using overridden mode. Same case for AUO 0x615c
> B116XAN06.1.
>
> Add such entries and use the hash of the EDID to match the panel needs the
> overridden modes.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/panel/panel-edp.c | 52 +++++++++++++++++++++++++++----
> 1 file changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index f6ddbaa103b5..42c430036846 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -21,6 +21,7 @@
> * DEALINGS IN THE SOFTWARE.
> */
>
> +#include <linux/crc32.h>
> #include <linux/debugfs.h>
> #include <linux/delay.h>
> #include <linux/gpio/consumer.h>
> @@ -213,6 +214,9 @@ struct edp_panel_entry {
> /** @panel_id: 32-bit ID for panel, encoded with drm_edid_encode_panel_id(). */
> u32 panel_id;
>
> + /** @panel_hash: the CRC32 hash of the EDID base block. */
> + u32 panel_hash;
> +
> /** @delay: The power sequencing delays needed for this panel. */
> const struct panel_delay *delay;
>
> @@ -758,13 +762,13 @@ static void panel_edp_parse_panel_timing_node(struct device *dev,
> dev_err(dev, "Reject override mode: No display_timing found\n");
> }
>
> -static const struct edp_panel_entry *find_edp_panel(u32 panel_id);
> +static const struct edp_panel_entry *find_edp_panel(u32 panel_id, u32 panel_hash);
>
> static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> {
> struct panel_desc *desc;
> void *base_block;
> - u32 panel_id;
> + u32 panel_id, panel_hash;
> char vend[4];
> u16 product_id;
> u32 reliable_ms = 0;
> @@ -796,15 +800,17 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> base_block = drm_edid_get_base_block(panel->ddc);
> if (base_block) {
> panel_id = drm_edid_get_panel_id(base_block);
> + panel_hash = crc32_le(~0, base_block, EDID_LENGTH) ^ 0xffffffff;
> kfree(base_block);
> } else {
> dev_err(dev, "Couldn't identify panel via EDID\n");
> ret = -EIO;
> goto exit;
> }
> +
> drm_edid_decode_panel_id(panel_id, vend, &product_id);
>
> - panel->detected_panel = find_edp_panel(panel_id);
> + panel->detected_panel = find_edp_panel(panel_id, panel_hash);
>
> /*
> * We're using non-optimized timings and want it really obvious that
> @@ -1006,6 +1012,19 @@ static const struct panel_desc auo_b101ean01 = {
> },
> };
>
> +static const struct drm_display_mode auo_b116xa3_mode = {
> + .clock = 70589,
> + .hdisplay = 1366,
> + .hsync_start = 1366 + 40,
> + .hsync_end = 1366 + 40 + 40,
> + .htotal = 1366 + 40 + 40 + 32,
> + .vdisplay = 768,
> + .vsync_start = 768 + 10,
> + .vsync_end = 768 + 10 + 12,
> + .vtotal = 768 + 10 + 12 + 6,
> + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> +};
> +
> static const struct drm_display_mode auo_b116xak01_mode = {
> .clock = 69300,
> .hdisplay = 1366,
> @@ -1926,11 +1945,13 @@ static const struct panel_delay delay_200_500_e50_po2e200 = {
> .delay = _delay \
> }
>
> -#define EDP_PANEL_ENTRY2(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name, _mode) \
> +#define EDP_PANEL_ENTRY2(vend_chr_0, vend_chr_1, vend_chr_2, product_id, \
> + _hash, _delay, _name, _mode) \
> { \
> .name = _name, \
> .panel_id = drm_edid_encode_panel_id(vend_chr_0, vend_chr_1, vend_chr_2, \
> product_id), \
> + .panel_hash = _hash, \
> .delay = _delay, \
> .override_edid_mode = _mode \
> }
> @@ -2077,13 +2098,32 @@ static const struct edp_panel_entry edp_panels[] = {
> { /* sentinal */ }
> };
>
> -static const struct edp_panel_entry *find_edp_panel(u32 panel_id)
> +/*
> + * Similar to edp_panels, this table lists panel variants that require using
> + * overridden modes but have the same panel id as one of the entries in edp_panels.
> + *
> + * Sort first by vendor, then by product ID.
> + */
> +static const struct edp_panel_entry edp_panels_variants[] = {

I'd suggest keeping these entries in the main table. Otherwise it
becomes harder to note, which setting gets used.

> + EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, 0xa9951478, &auo_b116xak01.delay,
> + "B116XAK01.0", &auo_b116xa3_mode),
> + EDP_PANEL_ENTRY2('A', 'U', 'O', 0x615c, 0x109b75b3, &delay_200_500_e50,
> + "B116XAN06.1", &auo_b116xa3_mode),
> +
> + { /* sentinal */ }
> +};
> +
> +static const struct edp_panel_entry *find_edp_panel(u32 panel_id, u32 panel_hash)
> {
> const struct edp_panel_entry *panel;
>
> - if (!panel_id)
> + if (!panel_id || !panel_hash)
> return NULL;
>
> + for (panel = edp_panels_variants; panel->panel_id; panel++)
> + if (panel->panel_id == panel_id && panel->panel_hash == panel_hash)
> + return panel;
> +
> for (panel = edp_panels; panel->panel_id; panel++)
> if (panel->panel_id == panel_id)
> return panel;
> --
> 2.44.0.rc0.258.g7320e95886-goog
>


--
With best wishes
Dmitry