When addressing Imre's comments I noticed:
error: ‘cnl_set_cdclk’ defined but not used [-Werror=unused-function]
static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
^
cc1: all warnings being treated as errors
Ville, since the original is yours I'd like your advise on how to proceed.
1. squash both
2. swap the patches and create a temporary emply cnl_set_cdclk
3. ?
Thanks,
Rodrigo.
On Mon, Jun 5, 2017 at 1:07 PM, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> On Mon, Jun 05, 2017 at 09:28:52PM +0300, Vivi, Rodrigo wrote:
>> On Mon, 2017-06-05 at 21:21 +0300, Imre Deak wrote:
>> > On Mon, Jun 05, 2017 at 09:04:26PM +0300, Ville Syrjälä wrote:
>> > > On Mon, Jun 05, 2017 at 05:59:02PM +0000, Vivi, Rodrigo wrote:
>> > > > On Fri, 2017-06-02 at 21:06 +0300, Imre Deak wrote:
>> > > > > > [...]
>> > > > > > +static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv,
>> > > > > > + struct intel_cdclk_state *cdclk_state)
>> > > > > > +{
>> > > > > > + u32 val;
>> > > > > > +
>> > > > > > + if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
>> > > > > > + cdclk_state->ref = 24000;
>> > > > > > + else
>> > > > > > + cdclk_state->ref = 19200;
>> > > > > > +
>> > > > > > + cdclk_state->vco = 0;
>> > > > > > +
>> > > > > > + val = I915_READ(BXT_DE_PLL_ENABLE);
>> > > > > > + if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
>> > > > > > + return;
>> > > > > > +
>> > > > > > + if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
>> > > > > > + return;
>> > > > > > +
>> > > > > > + cdclk_state->vco = (val & CNL_CDCLK_PLL_RATIO_MASK) * cdclk_state->ref;
>> > > > > > +}
>> > > > > > +
>> > > > > > +static void cnl_get_cdclk(struct drm_i915_private *dev_priv,
>> > > > > > + struct intel_cdclk_state *cdclk_state)
>> > > > > > +{
>> > > > > > + u32 divider;
>> > > > > > + int div;
>> > > > > > +
>> > > > > > + cnl_cdclk_pll_update(dev_priv, cdclk_state);
>> > > > >
>> > > > > The other platforms set cdclk to the ref clock here, not sure
>> > > > > if it's ok to leave it uninited. With that change it looks ok:
>> > > >
>> > > > Not sure how to address this here...
>> > > > I see bxt and skl using the cdclk_state here...
>> > >
>> > > Assuming refclk is the bypass clock then just doing what the earlier
>> > > platforms do would be correct.
>> >
>> > Yes, according to Bspec the CDCLK freq is the refclock freq when the PLL
>> > is disabled.
>>
>> So, do I need to change anything?
>
> Yes, add the following after cnl_cdclk_pll_update() as done on other
> gen9+ platforms:
>
> cdclk_state->cdclk = cdclk_state->ref;
>
> --Imre
>
>>
>> >
>> > > IIRC there was some platform where the bypass clock wasn't the refclk,
>> > > but that was perhaps some future thing. Either way, whatever the
>> > > bypass clock is we will want the readout to correctly reflect it when
>> > > the PLL is off.
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
