Re: [PATCH v3 1/2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 14 Feb 2025, Egor Vorontsov <sdoregor@sdore.me> wrote:
> Some newer high refresh rate consumer monitors (including those by Samsung)
> make use of DisplayID 2.1 timing blocks in their EDID data, notably for
> their highest refresh rate modes. Such modes won't be available as of now.
>
> Implement partial support for such blocks in order to enable native
> support of HRR modes of most such monitors for users without having to rely
> on EDID patching/override (or need thereof).
>
> Closes: https://gitlab.freedesktop.org/drm/misc/kernel/-/issues/55
> Suggested-by: Maximilian Boße <max@bosse.io>
> Signed-off-by: Egor Vorontsov <sdoregor@sdore.me>

PS. I bounced the messages to intel-gfx to have Intel CI run these.

> ---
>  drivers/gpu/drm/drm_displayid_internal.h | 13 +++++
>  drivers/gpu/drm/drm_edid.c               | 63 ++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
> index aee1b86a73c1..84831ecfdb6e 100644
> --- a/drivers/gpu/drm/drm_displayid_internal.h
> +++ b/drivers/gpu/drm/drm_displayid_internal.h
> @@ -66,6 +66,7 @@ struct drm_edid;
>  #define DATA_BLOCK_2_STEREO_DISPLAY_INTERFACE	0x27
>  #define DATA_BLOCK_2_TILED_DISPLAY_TOPOLOGY	0x28
>  #define DATA_BLOCK_2_CONTAINER_ID		0x29
> +#define DATA_BLOCK_2_TYPE_10_FORMULA_TIMING	0x2a
>  #define DATA_BLOCK_2_VENDOR_SPECIFIC		0x7e
>  #define DATA_BLOCK_2_CTA_DISPLAY_ID		0x81
>  
> @@ -129,6 +130,18 @@ struct displayid_detailed_timing_block {
>  	struct displayid_detailed_timings_1 timings[];
>  };
>  
> +struct displayid_formula_timings_9 {
> +	u8 flags;
> +	__le16 hactive;
> +	__le16 vactive;
> +	u8 vrefresh;
> +} __packed;
> +
> +struct displayid_formula_timing_block {
> +	struct displayid_block base;
> +	struct displayid_formula_timings_9 timings[];
> +} __packed;
> +
>  #define DISPLAYID_VESA_MSO_OVERLAP	GENMASK(3, 0)
>  #define DISPLAYID_VESA_MSO_MODE		GENMASK(6, 5)
>  
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 13bc4c290b17..03edf0e1598e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6833,6 +6833,66 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
>  	return num_modes;
>  }
>  
> +static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *dev,
> +							   const struct displayid_formula_timings_9 *timings,
> +							   bool type_10)
> +{
> +	struct drm_display_mode *mode;
> +	u16 hactive = le16_to_cpu(timings->hactive) + 1;
> +	u16 vactive = le16_to_cpu(timings->vactive) + 1;
> +	u8 timing_formula = timings->flags & 0x7;
> +
> +	/* TODO: support RB-v2 & RB-v3 */
> +	if (timing_formula > 1)
> +		return NULL;
> +
> +	/* TODO: support video-optimized refresh rate */
> +	if (timings->flags & (1 << 4))
> +		drm_dbg_kms(dev, "Fractional vrefresh is not implemented, proceeding with non-video-optimized refresh rate");
> +
> +	mode = drm_cvt_mode(dev, hactive, vactive, timings->vrefresh + 1, timing_formula == 1, false, false);
> +	if (!mode)
> +		return NULL;
> +
> +	/* TODO: interpret S3D flags */
> +
> +	mode->type = DRM_MODE_TYPE_DRIVER;
> +	drm_mode_set_name(mode);
> +
> +	return mode;
> +}
> +
> +static int add_displayid_formula_modes(struct drm_connector *connector,
> +				       const struct displayid_block *block)
> +{
> +	const struct displayid_formula_timing_block *formula_block = (struct displayid_formula_timing_block *)block;
> +	int num_timings;
> +	struct drm_display_mode *newmode;
> +	int num_modes = 0;
> +	bool type_10 = block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING;
> +	int timing_size = 6 + ((formula_block->base.rev & 0x70) >> 4);
> +
> +	/* extended blocks are not supported yet */
> +	if (timing_size != 6)
> +		return 0;
> +
> +	if (block->num_bytes % timing_size)
> +		return 0;
> +
> +	num_timings = block->num_bytes / timing_size;
> +	for (int i = 0; i < num_timings; i++) {
> +		const struct displayid_formula_timings_9 *timings = &formula_block->timings[i];
> +
> +		newmode = drm_mode_displayid_formula(connector->dev, timings, type_10);
> +		if (!newmode)
> +			continue;
> +
> +		drm_mode_probed_add(connector, newmode);
> +		num_modes++;
> +	}
> +	return num_modes;
> +}
> +
>  static int add_displayid_detailed_modes(struct drm_connector *connector,
>  					const struct drm_edid *drm_edid)
>  {
> @@ -6845,6 +6905,9 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
>  		if (block->tag == DATA_BLOCK_TYPE_1_DETAILED_TIMING ||
>  		    block->tag == DATA_BLOCK_2_TYPE_7_DETAILED_TIMING)
>  			num_modes += add_displayid_detailed_1_modes(connector, block);
> +		else if (block->tag == DATA_BLOCK_2_TYPE_9_FORMULA_TIMING ||
> +			 block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING)
> +			num_modes += add_displayid_formula_modes(connector, block);
>  	}
>  	displayid_iter_end(&iter);

-- 
Jani Nikula, Intel