drm/edid: Fix off-by-one in DispID DTD pixel clock
diff mbox series

Message ID 20200423151743.18767-1-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • drm/edid: Fix off-by-one in DispID DTD pixel clock
Related show

Commit Message

Ville Syrjälä April 23, 2020, 3:17 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The DispID DTD pixel clock is documented as:
"00 00 00 h → FF FF FF h | Pixel clock ÷ 10,000 0.01 → 167,772.16 Mega Pixels per Sec"
Which seems to imply that we to add one to the raw value.

Reality seems to agree as there are tiled displays in the wild
which currently show a 10kHz difference in the pixel clock
between the tiles (one tile gets its mode from the base EDID,
the other from the DispID block).

Cc: stable@vger.kernel.org
References: https://gitlab.freedesktop.org/drm/intel/-/issues/27
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Manasi Navare April 23, 2020, 6:45 p.m. UTC | #1
On Thu, Apr 23, 2020 at 06:17:43PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The DispID DTD pixel clock is documented as:
> "00 00 00 h → FF FF FF h | Pixel clock ÷ 10,000 0.01 → 167,772.16 Mega Pixels per Sec"
> Which seems to imply that we to add one to the raw value.
> 
> Reality seems to agree as there are tiled displays in the wild
> which currently show a 10kHz difference in the pixel clock
> between the tiles (one tile gets its mode from the base EDID,
> the other from the DispID block).
> 
> Cc: stable@vger.kernel.org
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/27
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes total sense,

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/drm_edid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 43b6ca364daa..544d2603f5fc 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5120,7 +5120,7 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d
>  	struct drm_display_mode *mode;
>  	unsigned pixel_clock = (timings->pixel_clock[0] |
>  				(timings->pixel_clock[1] << 8) |
> -				(timings->pixel_clock[2] << 16));
> +				(timings->pixel_clock[2] << 16)) + 1;
>  	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;
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Sasha Levin April 26, 2020, 3:03 p.m. UTC | #2
Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.

v5.6.7: Build OK!
v5.4.35: Build OK!
v4.19.118: Build OK!
v4.14.177: Build OK!
v4.9.220: Build OK!
v4.4.220: Failed to apply! Possible dependencies:
    3a4a2ea39f86 ("drm/displayid: Iterate over all DisplayID blocks")
    5e546cd5b3bc ("drm/edid: move displayid tiled block parsing into separate function.")
    a39ed680bddb ("drm/edid: add displayid detailed 1 timings to the modelist. (v1.1)")
    c97291774c1b ("drm/edid: move displayid validation to it's own function.")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 43b6ca364daa..544d2603f5fc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5120,7 +5120,7 @@  static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d
 	struct drm_display_mode *mode;
 	unsigned pixel_clock = (timings->pixel_clock[0] |
 				(timings->pixel_clock[1] << 8) |
-				(timings->pixel_clock[2] << 16));
+				(timings->pixel_clock[2] << 16)) + 1;
 	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;