From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 3/3] drm/i915: Simplify watermark/init_clock_gating setup
Date: Tue, 7 Jan 2014 21:53:02 +0200 [thread overview]
Message-ID: <20140107195301.GG4800@intel.com> (raw)
In-Reply-To: <CA+gsUGQFxGgpR_oFpbzJpYA7TrggC7A+i3MKRZSbLSHD4zhFmQ@mail.gmail.com>
On Tue, Jan 07, 2014 at 05:34:40PM -0200, Paulo Zanoni wrote:
> 2014/1/7 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Avoid duplicating the same piece of code several times by separating
> > the watemark vfunc setup from the init_clock_gating vfunc setup on PCH
> > platforms.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 78 +++++++++--------------------------------
> > 1 file changed, 16 insertions(+), 62 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 04e1e29..a177a93 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5575,73 +5575,27 @@ void intel_init_pm(struct drm_device *dev)
> > if (HAS_PCH_SPLIT(dev)) {
> > intel_setup_wm_latency(dev);
> >
> > - if (IS_GEN5(dev)) {
> > - if (dev_priv->wm.pri_latency[1] &&
> > - dev_priv->wm.spr_latency[1] &&
> > - dev_priv->wm.cur_latency[1]) {
> > - dev_priv->display.update_wm = ilk_update_wm;
> > - dev_priv->display.update_sprite_wm =
> > - ilk_update_sprite_wm;
> > - } else {
> > - DRM_DEBUG_KMS("Failed to get proper latency. "
> > - "Disable CxSR\n");
> > - dev_priv->display.update_wm = NULL;
> > - }
> > + if ((IS_GEN5(dev) && dev_priv->wm.pri_latency[1] &&
> > + dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) ||
> > + (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
> > + dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
> > + dev_priv->display.update_wm = ilk_update_wm;
> > + dev_priv->display.update_sprite_wm = ilk_update_sprite_wm;
>
> My tiny little brain doesn't remember why on gen5 we check for
> latency[1] instead of latency[0]. I know this is not the goal if your
> patch, but maybe a follow-up patch adding a little comment would be
> nice :)
I did it that way since the latency[0] values are hardcoded on ILK.
So we can't tell from latency[0] values whether the BIOS programmed
the latency values or not. I had an idea to kill these checks, and
possibly just dynamically calculate the max level for which we have
latency data. But that's for another patch.
>
> For the 3 patches on the series:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
>
> > + } else {
> > + DRM_DEBUG_KMS("Failed to read display plane latency. "
> > + "Disable CxSR\n");
> > + }
> > +
> > + if (IS_GEN5(dev))
> > dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
> > - } else if (IS_GEN6(dev)) {
> > - if (dev_priv->wm.pri_latency[0] &&
> > - dev_priv->wm.spr_latency[0] &&
> > - dev_priv->wm.cur_latency[0]) {
> > - dev_priv->display.update_wm = ilk_update_wm;
> > - dev_priv->display.update_sprite_wm =
> > - ilk_update_sprite_wm;
> > - } else {
> > - DRM_DEBUG_KMS("Failed to read display plane latency. "
> > - "Disable CxSR\n");
> > - dev_priv->display.update_wm = NULL;
> > - }
> > + else if (IS_GEN6(dev))
> > dev_priv->display.init_clock_gating = gen6_init_clock_gating;
> > - } else if (IS_IVYBRIDGE(dev)) {
> > - if (dev_priv->wm.pri_latency[0] &&
> > - dev_priv->wm.spr_latency[0] &&
> > - dev_priv->wm.cur_latency[0]) {
> > - dev_priv->display.update_wm = ilk_update_wm;
> > - dev_priv->display.update_sprite_wm =
> > - ilk_update_sprite_wm;
> > - } else {
> > - DRM_DEBUG_KMS("Failed to read display plane latency. "
> > - "Disable CxSR\n");
> > - dev_priv->display.update_wm = NULL;
> > - }
> > + else if (IS_IVYBRIDGE(dev))
> > dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
> > - } else if (IS_HASWELL(dev)) {
> > - if (dev_priv->wm.pri_latency[0] &&
> > - dev_priv->wm.spr_latency[0] &&
> > - dev_priv->wm.cur_latency[0]) {
> > - dev_priv->display.update_wm = ilk_update_wm;
> > - dev_priv->display.update_sprite_wm =
> > - ilk_update_sprite_wm;
> > - } else {
> > - DRM_DEBUG_KMS("Failed to read display plane latency. "
> > - "Disable CxSR\n");
> > - dev_priv->display.update_wm = NULL;
> > - }
> > + else if (IS_HASWELL(dev))
> > dev_priv->display.init_clock_gating = haswell_init_clock_gating;
> > - } else if (INTEL_INFO(dev)->gen == 8) {
> > - if (dev_priv->wm.pri_latency[0] &&
> > - dev_priv->wm.spr_latency[0] &&
> > - dev_priv->wm.cur_latency[0]) {
> > - dev_priv->display.update_wm = ilk_update_wm;
> > - dev_priv->display.update_sprite_wm =
> > - ilk_update_sprite_wm;
> > - } else {
> > - DRM_DEBUG_KMS("Failed to read display plane latency. "
> > - "Disable CxSR\n");
> > - dev_priv->display.update_wm = NULL;
> > - }
> > + else if (INTEL_INFO(dev)->gen == 8)
> > dev_priv->display.init_clock_gating = gen8_init_clock_gating;
> > - } else
> > - dev_priv->display.update_wm = NULL;
> > } else if (IS_VALLEYVIEW(dev)) {
> > dev_priv->display.update_wm = valleyview_update_wm;
> > dev_priv->display.init_clock_gating =
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2014-01-07 19:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-07 14:14 [PATCH 0/3] drm/i915: BDW watermark fixes ville.syrjala
2014-01-07 14:14 ` [PATCH 1/3] drm/i915: Fix watermark code for BDW ville.syrjala
2014-01-07 14:14 ` [PATCH 2/3] drm/i915: Enable watermarks " ville.syrjala
2014-01-07 14:14 ` [PATCH 3/3] drm/i915: Simplify watermark/init_clock_gating setup ville.syrjala
2014-01-07 19:34 ` Paulo Zanoni
2014-01-07 19:53 ` Ville Syrjälä [this message]
2014-01-07 21:47 ` Daniel Vetter
2014-01-08 9:17 ` Ville Syrjälä
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140107195301.GG4800@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=przanoni@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox