| Message ID | 1438238978-10638-2-git-send-email-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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). > +{ > + 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. > + 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. Is this correct for gen9+? -Chris
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@intel.com> > > --- > > 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 >
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) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + int max_cdclk_freq = dev_priv->max_cdclk_freq; + int max_dotclk_freq; + + if (IS_BROADWELL(dev) || IS_CHERRYVIEW(dev)) + 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)) + max_dotclk_freq = DIV_ROUND_UP(2 * max_cdclk_freq * 100, 90); + else + max_dotclk_freq = max_cdclk_freq; + + return max_dotclk_freq; +} + static void intel_update_max_cdclk(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -5298,6 +5316,8 @@ static void intel_update_max_cdclk(struct drm_device *dev) dev_priv->max_cdclk_freq = dev_priv->cdclk_freq; } + dev_priv->max_dotclk = intel_update_max_dotclk(dev); + DRM_DEBUG_DRIVER("Max CD clock rate: %d kHz\n", dev_priv->max_cdclk_freq); }
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@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+)