On Thu, 2015-07-30 at 08:00 +0100, Chris Wilson wrote:
> On Thu, Jul 30, 2015 at 09:49:28AM +0300, Mika Kahola wrote:
> > Store max dotclock into dev_priv structure so we are able
> > to filter out the modes that are not supported by our
> > platforms.
> >
> > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 04aa34a..1f69211b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1777,6 +1777,7 @@ struct drm_i915_private {
> > unsigned int fsb_freq, mem_freq, is_ddr3;
> > unsigned int skl_boot_cdclk;
> > unsigned int cdclk_freq, max_cdclk_freq;
> > + unsigned int max_dotclk;
> > unsigned int hpll_freq;
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 43b0f17..9031261 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5259,6 +5259,24 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
> > modeset_put_power_domains(dev_priv, put_domains[i]);
> > }
> >
> > +static int intel_update_max_dotclk(struct drm_device *dev)
>
> You don't update max dotclck, you are computing it. The caller is the
> one storing it dev_priv->max_dotclk (and so is the one actually doing
> the update).
>
True. I'll fix the poor naming of that routine.
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
>
> So why did you pass in dev if we never use it?
>
> > + int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > + int max_dotclk_freq;
> > +
> > + if (IS_BROADWELL(dev) || IS_CHERRYVIEW(dev))
>
> We already have dev_priv, so please stop doing dev->dev_priv over and
> over again.
>
Yeah, that's pretty much unnecessary, so removing this one for the next
series of patches.
> > + max_dotclk_freq = DIV_ROUND_UP(max_cdclk_freq * 100, 95);
> > + else if (IS_VALLEYVIEW(dev))
> > + max_dotclk_freq = DIV_ROUND_UP(max_cdclk_freq * 100, 90);
> > + else if (IS_GEN2(dev) || IS_GEN3(dev))
>
> If you reverse this pair and do
>
> else if (INTEL_INFO(dev)->gen > 3)
> max_dotclk_freq = max_cdclk_freq;
> else
> max_dotclk_freq = DIV_ROUND_UP(2 * max_cdclk_freq * 100, 90);
>
> Then the chain is mostly ordered in most-recent to oldest, always
> helpful for the next person.
>
I'll apply this change as well on the next patch series.
> Is this correct for gen9+?
This is something I need to double check. My current understanding is
that we should be able to support dot clock up to cd clock frequency
from HSW+ onwards. Actually, I did missed the Ville's comment to limit
dot clock to 90% for older platforms so I need to add this to the next
series as well.
Thanks again for valuable comments.
Cheers,
Mika
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
