Re: [PATCH] drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock difference

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 16, 2015 at 09:05:12PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> Rather than using drm_match_cea_mode() to see if the EDID detailed
> timings are supposed to represent one of the CEA/HDMI modes, add a
> special version of that function that takes in an explicit clock
> tolerance value (in kHz). When looking at the detailed timings specify
> the tolerance as 5kHz due to the 10kHz clock resolution limit inherent
> in detailed timings.
> 
> drm_match_cea_mode() uses the normal KHZ2PICOS() matching of clocks,
> which only allows smaller errors for lower clocks (eg. for 25200 it
> won't allow any error) and a bigger error for higher clocks (eg. for
> 297000 it actually matches 296913-297000). So it doesn't really match
> what we want for the fixup. Using the explicit +-5kHz is much better
> for this use case.
> 
> Not sure if we should change the normal mode matching to also use
> something else besides KHZ2PICOS() since it allows a different
> proportion of error depending on the clock. I believe VESA CVT
> allows a maximum deviation of .5%, so using that for normal mode
> matching might be a good idea?

Ping. Any thoughts from anyone?

> 
> Cc: Adam Jackson <ajax@xxxxxxxxxx>
> Tested-by: nathan.d.ciobanu@xxxxxxxxxxxxxxx
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92217
> Fixes: fa3a7340eaa1 ("drm/edid: Fix up clock for CEA/HDMI modes specified via detailed timings")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_edid.c  | 62 +++++++++++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/drm_modes.c | 19 +++++++++++++-
>  include/drm/drm_modes.h     |  2 ++
>  3 files changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index d5d2c03fd136..c214f1246cb4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2545,6 +2545,33 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode)
>  	return clock;
>  }
>  
> +static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match,
> +					     unsigned int clock_tolerance)
> +{
> +	u8 mode;
> +
> +	if (!to_match->clock)
> +		return 0;
> +
> +	for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) {
> +		const struct drm_display_mode *cea_mode = &edid_cea_modes[mode];
> +		unsigned int clock1, clock2;
> +
> +		/* Check both 60Hz and 59.94Hz */
> +		clock1 = cea_mode->clock;
> +		clock2 = cea_mode_alternate_clock(cea_mode);
> +
> +		if (abs(to_match->clock - clock1) > clock_tolerance &&
> +		    abs(to_match->clock - clock2) > clock_tolerance)
> +			continue;
> +
> +		if (drm_mode_equal_no_clocks(to_match, cea_mode))
> +			return mode + 1;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_match_cea_mode - look for a CEA mode matching given mode
>   * @to_match: display mode
> @@ -2609,6 +2636,33 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
>  	return cea_mode_alternate_clock(hdmi_mode);
>  }
>  
> +static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match,
> +					      unsigned int clock_tolerance)
> +{
> +	u8 mode;
> +
> +	if (!to_match->clock)
> +		return 0;
> +
> +	for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) {
> +		const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode];
> +		unsigned int clock1, clock2;
> +
> +		/* Make sure to also match alternate clocks */
> +		clock1 = hdmi_mode->clock;
> +		clock2 = hdmi_mode_alternate_clock(hdmi_mode);
> +
> +		if (abs(to_match->clock - clock1) > clock_tolerance &&
> +		    abs(to_match->clock - clock2) > clock_tolerance)
> +			continue;
> +
> +		if (drm_mode_equal_no_clocks(to_match, hdmi_mode))
> +			return mode + 1;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * drm_match_hdmi_mode - look for a HDMI mode matching given mode
>   * @to_match: display mode
> @@ -3119,14 +3173,18 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>  	u8 mode_idx;
>  	const char *type;
>  
> -	mode_idx = drm_match_cea_mode(mode) - 1;
> +	/*
> +	 * allow 5kHz clock difference either way to account for
> +	 * the 10kHz clock resolution limit of detailed timings.
> +	 */
> +	mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1;
>  	if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
>  		type = "CEA";
>  		cea_mode = &edid_cea_modes[mode_idx];
>  		clock1 = cea_mode->clock;
>  		clock2 = cea_mode_alternate_clock(cea_mode);
>  	} else {
> -		mode_idx = drm_match_hdmi_mode(mode) - 1;
> +		mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1;
>  		if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
>  			type = "HDMI";
>  			cea_mode = &edid_4k_modes[mode_idx];
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 9480464434cf..4d2a24121dc2 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -917,13 +917,30 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ
>  	} else if (mode1->clock != mode2->clock)
>  		return false;
>  
> +	return drm_mode_equal_no_clocks(mode1, mode2);
> +}
> +EXPORT_SYMBOL(drm_mode_equal);
> +
> +/**
> + * drm_mode_equal_no_clocks - test modes for equality
> + * @mode1: first mode
> + * @mode2: second mode
> + *
> + * Check to see if @mode1 and @mode2 are equivalent, but
> + * don't check the pixel clocks.
> + *
> + * Returns:
> + * True if the modes are equal, false otherwise.
> + */
> +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2)
> +{
>  	if ((mode1->flags & DRM_MODE_FLAG_3D_MASK) !=
>  	    (mode2->flags & DRM_MODE_FLAG_3D_MASK))
>  		return false;
>  
>  	return drm_mode_equal_no_clocks_no_stereo(mode1, mode2);
>  }
> -EXPORT_SYMBOL(drm_mode_equal);
> +EXPORT_SYMBOL(drm_mode_equal_no_clocks);
>  
>  /**
>   * drm_mode_equal_no_clocks_no_stereo - test modes for equality
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 08a8cac9e555..f9115aee43f4 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -222,6 +222,8 @@ struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev,
>  					    const struct drm_display_mode *mode);
>  bool drm_mode_equal(const struct drm_display_mode *mode1,
>  		    const struct drm_display_mode *mode2);
> +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1,
> +			      const struct drm_display_mode *mode2);
>  bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
>  					const struct drm_display_mode *mode2);
>  
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux