Re: [PATCH 5/5] drm/edid: add displayid detailed 1 timings to the modelist.

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

On Tue, 03 May 2016, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> The tiled 5K Dell monitor appears to be hiding it's tiled mode
> inside the displayid timings block, this patch parses this
> blocks and adds the modes to the modelist.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95207
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 105 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_displayid.h |  17 +++++++
>  2 files changed, 122 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e85d828..aca9e25 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3925,6 +3925,110 @@ static int validate_displayid(u8 *displayid, int length, int idx)
>  	return 0;
>  }
>  
> +static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
> +							    struct displayid_detailed_timings_1 *timings)
> +{
> +	struct drm_display_mode *mode;
> +	unsigned pixel_clock = (timings->pixel_clock[0] |
> +				(timings->pixel_clock[1] << 8) |
> +				(timings->pixel_clock[2] << 16));
> +	unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1;
> +	unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1;
> +	unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1;
> +	unsigned hsync_width = (timings->hsw[0] | timings->hsw[1] << 8) + 1;
> +	unsigned vactive = (timings->vactive[0] | timings->vactive[1] << 8) + 1;
> +	unsigned vblank = (timings->vblank[0] | timings->vblank[1] << 8) + 1;
> +	unsigned vsync = (timings->vsync[0] | (timings->vsync[1] & 0x7f) << 8) + 1;
> +	unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1;
> +	bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
> +	bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
> +	mode = drm_mode_create(dev);
> +	if (!mode)
> +		return NULL;
> +
> +	mode->clock = pixel_clock * 10;
> +	mode->hdisplay = hactive;
> +	mode->hsync_start = mode->hdisplay + hsync;
> +	mode->hsync_end = mode->hsync_start + hsync_width;
> +	mode->htotal = mode->hdisplay + hblank;
> +
> +	mode->vdisplay = vactive;
> +	mode->vsync_start = mode->vdisplay + vsync;
> +	mode->vsync_end = mode->vsync_start + vsync_width;
> +	mode->vtotal = mode->vdisplay + vblank;
> +
> +	mode->flags = 0;
> +	mode->flags |= hsync_positive ? DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
> +	mode->flags |= vsync_positive ? DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
> +	mode->type = DRM_MODE_TYPE_DRIVER;
> +
> +	if (timings->flags & 0x80)
> +		mode->type |= DRM_MODE_TYPE_PREFERRED;
> +	mode->vrefresh = drm_mode_vrefresh(mode);
> +	drm_mode_set_name(mode);
> +
> +	return mode;
> +}
> +
> +static int add_displayid_detailed_1_modes(struct drm_connector *connector,
> +					  struct displayid_block *block)
> +{
> +	struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block;
> +	int i;
> +	int num_timings;
> +	struct drm_display_mode *newmode;
> +	int num_modes = 0;
> +	/* blocks must be multiple of 20 bytes length */
> +	if (block->num_bytes % 20)
> +		return 0;
> +
> +	num_timings = block->num_bytes / 20;
> +	for (i = 0; i < num_timings; i++) {
> +		struct displayid_detailed_timings_1 *timings = &det->timings[i];
> +
> +		newmode = drm_mode_displayid_detailed(connector->dev, timings);
> +		if (!newmode)
> +			continue;
> +
> +		drm_mode_probed_add(connector, newmode);
> +		num_modes++;
> +	}
> +	return num_modes;
> +}
> +
> +static int add_displayid_detailed_modes(struct drm_connector *connector,
> +					struct edid *edid)
> +{
> +	u8 *displayid;
> +	int ret;
> +	int idx = 1;
> +	int length = EDID_LENGTH;
> +	struct displayid_block *block;
> +	int num_modes = 0;
> +
> +	displayid = drm_find_displayid_extension(edid);
> +	if (!displayid)
> +		return 0;
> +
> +	ret = validate_displayid(displayid, length, idx);
> +	if (ret)
> +		return 0;
> +
> +	idx += sizeof(struct displayid_hdr);
> +	while (block = (struct displayid_block *)&displayid[idx],
> +	       idx + sizeof(struct displayid_block) <= length &&
> +	       idx + sizeof(struct displayid_block) + block->num_bytes <= length &&
> +	       block->num_bytes > 0) {
> +		idx += block->num_bytes + sizeof(struct displayid_block);
> +		switch (block->tag) {
> +		case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
> +			num_modes += add_displayid_detailed_1_modes(connector, block);
> +			break;
> +		}
> +	}
> +	return num_modes;
> +}
> +
>  /**
>   * drm_add_edid_modes - add modes from EDID data, if available
>   * @connector: connector we're probing
> @@ -3970,6 +4074,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>  	num_modes += add_established_modes(connector, edid);
>  	num_modes += add_cea_modes(connector, edid);
>  	num_modes += add_alternate_cea_modes(connector, edid);
> +	num_modes += add_displayid_detailed_modes(connector, edid);
>  	if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
>  		num_modes += add_inferred_modes(connector, edid);
>  
> diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
> index 042f9fc..edef51d 100644
> --- a/include/drm/drm_displayid.h
> +++ b/include/drm/drm_displayid.h
> @@ -75,4 +75,21 @@ struct displayid_tiled_block {
>  	u8 topology_id[8];
>  } __packed;
>  
> +struct displayid_detailed_timings_1 {
> +	u8 pixel_clock[3];
> +	u8 flags;
> +	u8 hactive[2];
> +	u8 hblank[2];
> +	u8 hsync[2];
> +	u8 hsw[2];
> +	u8 vactive[2];
> +	u8 vblank[2];
> +	u8 vsync[2];
> +	u8 vsw[2];

An alternative would be to declare these fields as __le16, and you could
read them in drm_mode_displayid_detailed() using le16_to_cpu().

Anyway, these structs should be __packed.

BR,
Jani.


> +};
> +
> +struct displayid_detailed_timing_block {
> +	struct displayid_block base;
> +	struct displayid_detailed_timings_1 timings[0];
> +};
>  #endif

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel