Re: [PATCH 7/8] drm/i915/lvds: Remove magic from PLL programming

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 7/8] drm/i915/lvds: Remove magic from PLL programming
Date: Wed, 26 Apr 2017 17:50:06 +0300	[thread overview]
Message-ID: <20170426145006.GX30290@intel.com> (raw)
In-Reply-To: <1493214013-15580-8-git-send-email-imre.deak@intel.com>

On Wed, Apr 26, 2017 at 04:40:12PM +0300, Imre Deak wrote:
> This looks like a left-over from enabling work. I don't have the
> specification to check whether we have to set
> CH7017_LVDS_PLL_FEEDBACK_DEFAULT_RESERVED, for now just keep things
> as-is, removing the magic so that static checkers don't complain.

The spec does list the top two bits as reserved with the default value
of 10b. I don't see any mention of how reserved bits should be handled
though. But I think I'd just change it to set them to the default value.

The whole thing just looks like an oversight in the original ddx commit,
whose commit message isn't all that helpful:

commit 04e936935f0b0045600241424f1d04a6721a2432
Author: Eric Anholt <eric@anholt.net>
Date:   Mon Oct 1 17:29:35 2007 -0700

    Bring the CH7017 driver closer to spec.

    This is also closer to what my hardware is programmed with, except for some
    very confusing off-by-one bugs in an unexpected direction.

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/dvo_ch7017.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c b/drivers/gpu/drm/i915/dvo_ch7017.c
> index b3c7c19..c0712a5 100644
> --- a/drivers/gpu/drm/i915/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/dvo_ch7017.c
> @@ -280,10 +280,13 @@ static void ch7017_mode_set(struct intel_dvo_device *dvo,
>  			(0 << CH7017_PHASE_DETECTOR_SHIFT);
>  	} else {
>  		outputs_enable = CH7017_LVDS_CHANNEL_A | CH7017_CHARGE_PUMP_HIGH;
> -		lvds_pll_feedback_div = CH7017_LVDS_PLL_FEEDBACK_DEFAULT_RESERVED |
> +		/*
> +		 * FIXME: Check if CH7017_LVDS_PLL_FEEDBACK_DEFAULT_RESERVED
> +		 * needs to be also set for the following.
> +		 */
> +		lvds_pll_feedback_div =
>  			(2 << CH7017_LVDS_PLL_FEED_BACK_DIVIDER_SHIFT) |
>  			(3 << CH7017_LVDS_PLL_FEED_FORWARD_DIVIDER_SHIFT);
> -		lvds_pll_feedback_div = 35;
>  		lvds_control_2 = (3 << CH7017_LOOP_FILTER_SHIFT) |
>  			(0 << CH7017_PHASE_DETECTOR_SHIFT);
>  		if (1) { /* XXX: dual channel panel detection.  Assume yes for now. */
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx