Re: [PATCH 04/12] drm: Add support for alternate clocks of 4k modes

[Simon Farnsworth <simon.farnsworth@onelan.co.uk>]

[-- Attachment #1.1: Type: text/plain, Size: 3808 bytes --]

A few minor things:

On Wednesday 14 August 2013 00:17:20 Damien Lespiau wrote:
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 68 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0faa08e..606335f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2409,6 +2409,54 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>  }
>  EXPORT_SYMBOL(drm_match_cea_mode);
>  
> +/*
> + * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
> + * specific block).
> + *
> + * It's almost like cea_mode_alternate_clock(), we just need to add an
> + * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
> + * one.
> + */
> +static unsigned int
> +hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
> +{
> +	if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
> +		return hdmi_mode->clock;
> +
> +	return cea_mode_alternate_clock(hdmi_mode);
> +}
> +
> +/*
> + * drm_match_cea_mode - look for a HDMI mode matching given mode

Here we document drm_match_cea_mode, but the function is drm_match_hdmi_mode.

> + * @to_match: display mode
> + *
> + * An HDMI mode is one defined in the HDMI vendor specific block.
> + *
> + * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
> + */
> +static u8 drm_match_hmdi_mode(const struct drm_display_mode *to_match)
                       ^^^^
Surely should be hdmi, not hmdi?
> +{
> +	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 ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
> +		     KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
> +		    drm_mode_equal_no_clocks(to_match, hdmi_mode))
> +			return mode + 1;
> +	}
> +	return 0;
> +}
> +
>  static int
>  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  {
> @@ -2426,18 +2474,26 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>  	 * with the alternate clock for certain CEA modes.
>  	 */
>  	list_for_each_entry(mode, &connector->probed_modes, head) {
> -		const struct drm_display_mode *cea_mode;
> +		const struct drm_display_mode *cea_mode = NULL;
>  		struct drm_display_mode *newmode;
> -		u8 cea_mode_idx = drm_match_cea_mode(mode) - 1;
> +		u8 mode_idx = drm_match_cea_mode(mode) - 1;
>  		unsigned int clock1, clock2;
>  
> -		if (cea_mode_idx >= ARRAY_SIZE(edid_cea_modes))
> -			continue;
> +		if (mode_idx < ARRAY_SIZE(edid_cea_modes)) {
> +			cea_mode = &edid_cea_modes[mode_idx];
> +			clock2 = cea_mode_alternate_clock(cea_mode);
> +		} else {
> +			mode_idx = drm_match_hmdi_mode(mode) - 1;
                                 ^^^^
Same typo here - hmdi for hdmi.

> +			if (mode_idx < ARRAY_SIZE(edid_4k_modes)) {
> +				cea_mode = &edid_4k_modes[mode_idx];
> +				clock2 = hdmi_mode_alternate_clock(cea_mode);
> +			}
> +		}
>  
> -		cea_mode = &edid_cea_modes[cea_mode_idx];
> +		if (!cea_mode)
> +			continue;
>  
>  		clock1 = cea_mode->clock;
> -		clock2 = cea_mode_alternate_clock(cea_mode);
>  
>  		if (clock1 == clock2)
>  			continue;
> 
-- 
Simon Farnsworth
Software Engineer
ONELAN Ltd
http://www.onelan.com

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel