On Thu, Feb 11, 2016 at 05:11:52PM -0800, Marc Herbert wrote:
> [I'm cheating and doing this code review with the author watching over my shoulder]
>
> On 11/02/16 15:22, clinton.a.taylor@xxxxxxxxx wrote:
> > From: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> >
> > Track VCO frequency of SKL instead of the boot CDCLK and allow modeset
> > to set cdclk based on the max required pixel clock based on VCO
> > selected.
>
> Nit: the main point shouldn't come second.
>
> > The vco should be tracked at the atomic level and all CRTCs updated if
> > the required vco is changed. At this time the eDP pll is configured
> > inside the encoder which has no visibility into the atomic state.
>
> should be -> is
>
>
> > When eDP v1.4 panel that require the 8640 vco are available this may need
> > to be investigated.
>
> Just say that 8640 can't be tested yet.
>
> > V1: initial version
> > V2: add vco tracking in intel_dp_compute_config(), rename
> > skl_boot_cdclk.
> > V3: rebase, V2 feedback not possible as encoders are not aware of
> > atomic.
> > V4: track target vco is atomic state. modeset all CRTCs if vco changes
> >
> > Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> > Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_ddi.c | 2 +-
> > drivers/gpu/drm/i915/intel_display.c | 97 +++++++++++++++++++++++++++++-----
> > drivers/gpu/drm/i915/intel_dp.c | 10 ++--
> > drivers/gpu/drm/i915/intel_drv.h | 4 ++
> > 5 files changed, 97 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8216665..f65dd1a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1822,7 +1822,7 @@ struct drm_i915_private {
> > int num_fence_regs; /* 8 on pre-965, 16 otherwise */
> >
> > unsigned int fsb_freq, mem_freq, is_ddr3;
> > - unsigned int skl_boot_cdclk;
> > + unsigned int skl_vco_freq;
> > unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;
> > unsigned int max_dotclk_freq;
> > unsigned int hpll_freq;
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 6d5b09f..285adab 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2958,7 +2958,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
> > int cdclk_freq;
> >
> > cdclk_freq = dev_priv->display.get_display_clock_speed(dev);
> > - dev_priv->skl_boot_cdclk = cdclk_freq;
> > + dev_priv->skl_vco_freq = skl_cdclk_get_vco(cdclk_freq);
>
> - skl_cdclk_get_vco() and skl_cdclk_frequencies[] should probably be renamed to:
> + skl_get_bios_cdclk_vco() and skl_bios_cdclk_frequencies[]
>
> to avoid confusion with the (different) mapping used in the new skl_modeset_calc_cdclk()
> function below.
Let's not. This stuff doesn't really have anything to do with the BIOS.
We just want to read out the current hardware state, nothing more.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
