Re: [PATCH 7/8] drm/i915: try harder to find WR PLL clock settings

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

On Wed, 08 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> If we don't find the exact refresh rate, go with the next one. This
> makes some modes work for me. They won't have the best settings, but
> will at least have something. Just returning from this function when
> we don't find the perfect settings does not help us at all.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ff03a3a..db242cf 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -267,7 +267,8 @@ struct wrpll_tmds_clock {
>  	u16 r2;		/* Reference divider */
>  };
>  
> -/* Table of matching values for WRPLL clocks programming for each frequency */
> +/* Table of matching values for WRPLL clocks programming for each frequency.
> + * The code assumes this table is sorted. */

I spotted some duplicate lines in the table. Perhaps you could remove
them while at it?

>  static const struct wrpll_tmds_clock wrpll_tmds_clock_table[] = {
>  	{19750,	38,	25,	18},
>  	{20000,	48,	32,	18},
> @@ -658,7 +659,7 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>  	int port = intel_hdmi->ddi_port;
>  	int pipe = intel_crtc->pipe;
> -	int p, n2, r2, valid=0;
> +	int p, n2, r2;
>  	u32 temp, i;
>  
>  	/* On Haswell, we need to enable the clocks and prepare DDI function to
> @@ -666,26 +667,20 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
>  	 */
>  	DRM_DEBUG_KMS("Preparing HDMI DDI mode for Haswell on port %c, pipe %c\n", port_name(port), pipe_name(pipe));
>  
> -	for (i=0; i < ARRAY_SIZE(wrpll_tmds_clock_table); i++) {
> -		if (crtc->mode.clock == wrpll_tmds_clock_table[i].clock) {
> -			p = wrpll_tmds_clock_table[i].p;
> -			n2 = wrpll_tmds_clock_table[i].n2;
> -			r2 = wrpll_tmds_clock_table[i].r2;
> +	for (i = 0; i < ARRAY_SIZE(wrpll_tmds_clock_table); i++)
> +		if (crtc->mode.clock <= wrpll_tmds_clock_table[i].clock)
> +			break;
>  
> -			DRM_DEBUG_KMS("WR PLL clock: found settings for %dKHz refresh rate: p=%d, n2=%d, r2=%d\n",
> -					crtc->mode.clock,
> -					p, n2, r2);

You drop this debug message. Is it intentional? The below DRM_INFO will
only be printed if an exact match is not found.

BR,
Jani.

> +	if (i == ARRAY_SIZE(wrpll_tmds_clock_table))
> +		i--;
>  
> -			valid = 1;
> -			break;
> -		}
> -	}
> +	if (wrpll_tmds_clock_table[i].clock != crtc->mode.clock)
> +		DRM_INFO("WR PLL: using settings for %dKHz on %dKHz mode\n",
> +			 wrpll_tmds_clock_table[i].clock, crtc->mode.clock);
>  
> -	if (!valid) {
> -		DRM_ERROR("Unable to find WR PLL clock settings for %dKHz refresh rate\n",
> -				crtc->mode.clock);
> -		return;
> -	}
> +	p = wrpll_tmds_clock_table[i].p;
> +	n2 = wrpll_tmds_clock_table[i].n2;
> +	r2 = wrpll_tmds_clock_table[i].r2;
>  
>  	/* Enable LCPLL if disabled */
>  	temp = I915_READ(LCPLL_CTL);
> -- 
> 1.7.11.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx