| Message ID | 20180626052536.15137-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 25, 2018 at 10:25:36PM -0700, Rodrigo Vivi wrote: > At some point we introduced the function pointers > on PSR code to help with VLV/CHV separation logic > because it had a different HW implementation from PSR. > > Since all converged to HSW PSR and we dropped the > VLV/CHV support, let's also kill the useless function > pointers and leave the code cleaner. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Yay for removing dead code! Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_drv.h | 8 ----- > drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++--------------------- > 2 files changed, 18 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6f08ab310118..2b684f431c60 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -631,14 +631,6 @@ struct i915_psr { > bool debug; > ktime_t last_entry_attempt; > ktime_t last_exit; > - > - void (*enable_source)(struct intel_dp *, > - const struct intel_crtc_state *); > - void (*disable_source)(struct intel_dp *, > - const struct intel_crtc_state *); > - void (*enable_sink)(struct intel_dp *); > - void (*activate)(struct intel_dp *); > - void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *); > }; > > enum intel_pch { > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index aea81ace854b..bd8e562212f7 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -278,8 +278,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) > } > } > > -static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, > - const struct intel_crtc_state *crtc_state) > +static void intel_psr_setup_vsc(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state) > { > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); > @@ -344,7 +344,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) > I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > } > > -static void hsw_psr_enable_sink(struct intel_dp *intel_dp) > +static void intel_psr_enable_sink(struct intel_dp *intel_dp) > { > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > struct drm_device *dev = dig_port->base.base.dev; > @@ -456,24 +456,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > I915_WRITE(EDP_PSR2_CTL, val); > } > > -static void hsw_psr_activate(struct intel_dp *intel_dp) > -{ > - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > - struct drm_device *dev = dig_port->base.base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - > - /* On HSW+ after we enable PSR on source it will activate it > - * as soon as it match configure idle_frame count. So > - * we just actually enable it here on activation time. > - */ > - > - /* psr1 and psr2 are mutually exclusive.*/ > - if (dev_priv->psr.psr2_enabled) > - hsw_activate_psr2(intel_dp); > - else > - hsw_activate_psr1(intel_dp); > -} > - > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > struct intel_crtc_state *crtc_state) > { > @@ -583,12 +565,17 @@ static void intel_psr_activate(struct intel_dp *intel_dp) > WARN_ON(dev_priv->psr.active); > lockdep_assert_held(&dev_priv->psr.lock); > > - dev_priv->psr.activate(intel_dp); > + /* psr1 and psr2 are mutually exclusive.*/ > + if (dev_priv->psr.psr2_enabled) > + hsw_activate_psr2(intel_dp); > + else > + hsw_activate_psr1(intel_dp); > + > dev_priv->psr.active = true; > } > > -static void hsw_psr_enable_source(struct intel_dp *intel_dp, > - const struct intel_crtc_state *crtc_state) > +static void intel_psr_enable_source(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state) > { > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > struct drm_device *dev = dig_port->base.base.dev; > @@ -666,9 +653,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, > dev_priv->psr.psr2_enabled = crtc_state->has_psr2; > dev_priv->psr.busy_frontbuffer_bits = 0; > > - dev_priv->psr.setup_vsc(intel_dp, crtc_state); > - dev_priv->psr.enable_sink(intel_dp); > - dev_priv->psr.enable_source(intel_dp, crtc_state); > + intel_psr_setup_vsc(intel_dp, crtc_state); > + intel_psr_enable_sink(intel_dp); > + intel_psr_enable_source(intel_dp, crtc_state); > dev_priv->psr.enabled = intel_dp; > > intel_psr_activate(intel_dp); > @@ -677,8 +664,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, > mutex_unlock(&dev_priv->psr.lock); > } > > -static void hsw_psr_disable(struct intel_dp *intel_dp, > - const struct intel_crtc_state *old_crtc_state) > +static void > +intel_psr_disable_source(struct intel_dp *intel_dp, > + const struct intel_crtc_state *old_crtc_state) > { > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > struct drm_device *dev = intel_dig_port->base.base.dev; > @@ -747,7 +735,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, > return; > } > > - dev_priv->psr.disable_source(intel_dp, old_crtc_state); > + intel_psr_disable_source(intel_dp, old_crtc_state); > > /* Disable PSR on Sink */ > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0); > @@ -986,11 +974,4 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > > INIT_WORK(&dev_priv->psr.work, intel_psr_work); > mutex_init(&dev_priv->psr.lock); > - > - dev_priv->psr.enable_source = hsw_psr_enable_source; > - dev_priv->psr.disable_source = hsw_psr_disable; > - dev_priv->psr.enable_sink = hsw_psr_enable_sink; > - dev_priv->psr.activate = hsw_psr_activate; > - dev_priv->psr.setup_vsc = hsw_psr_setup_vsc; > - > } > -- > 2.17.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jun 26, 2018 at 10:27:49AM +0200, Daniel Vetter wrote: > On Mon, Jun 25, 2018 at 10:25:36PM -0700, Rodrigo Vivi wrote: > > At some point we introduced the function pointers > > on PSR code to help with VLV/CHV separation logic > > because it had a different HW implementation from PSR. > > > > Since all converged to HSW PSR and we dropped the > > VLV/CHV support, let's also kill the useless function > > pointers and leave the code cleaner. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Yay for removing dead code! > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> thanks, pushed to dinq. > > > --- > > drivers/gpu/drm/i915/i915_drv.h | 8 ----- > > drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++--------------------- > > 2 files changed, 18 insertions(+), 45 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 6f08ab310118..2b684f431c60 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -631,14 +631,6 @@ struct i915_psr { > > bool debug; > > ktime_t last_entry_attempt; > > ktime_t last_exit; > > - > > - void (*enable_source)(struct intel_dp *, > > - const struct intel_crtc_state *); > > - void (*disable_source)(struct intel_dp *, > > - const struct intel_crtc_state *); > > - void (*enable_sink)(struct intel_dp *); > > - void (*activate)(struct intel_dp *); > > - void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *); > > }; > > > > enum intel_pch { > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > > index aea81ace854b..bd8e562212f7 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -278,8 +278,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) > > } > > } > > > > -static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, > > - const struct intel_crtc_state *crtc_state) > > +static void intel_psr_setup_vsc(struct intel_dp *intel_dp, > > + const struct intel_crtc_state *crtc_state) > > { > > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); > > @@ -344,7 +344,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) > > I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > > } > > > > -static void hsw_psr_enable_sink(struct intel_dp *intel_dp) > > +static void intel_psr_enable_sink(struct intel_dp *intel_dp) > > { > > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > struct drm_device *dev = dig_port->base.base.dev; > > @@ -456,24 +456,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > > I915_WRITE(EDP_PSR2_CTL, val); > > } > > > > -static void hsw_psr_activate(struct intel_dp *intel_dp) > > -{ > > - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > - struct drm_device *dev = dig_port->base.base.dev; > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - > > - /* On HSW+ after we enable PSR on source it will activate it > > - * as soon as it match configure idle_frame count. So > > - * we just actually enable it here on activation time. > > - */ > > - > > - /* psr1 and psr2 are mutually exclusive.*/ > > - if (dev_priv->psr.psr2_enabled) > > - hsw_activate_psr2(intel_dp); > > - else > > - hsw_activate_psr1(intel_dp); > > -} > > - > > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > > struct intel_crtc_state *crtc_state) > > { > > @@ -583,12 +565,17 @@ static void intel_psr_activate(struct intel_dp *intel_dp) > > WARN_ON(dev_priv->psr.active); > > lockdep_assert_held(&dev_priv->psr.lock); > > > > - dev_priv->psr.activate(intel_dp); > > + /* psr1 and psr2 are mutually exclusive.*/ > > + if (dev_priv->psr.psr2_enabled) > > + hsw_activate_psr2(intel_dp); > > + else > > + hsw_activate_psr1(intel_dp); > > + > > dev_priv->psr.active = true; > > } > > > > -static void hsw_psr_enable_source(struct intel_dp *intel_dp, > > - const struct intel_crtc_state *crtc_state) > > +static void intel_psr_enable_source(struct intel_dp *intel_dp, > > + const struct intel_crtc_state *crtc_state) > > { > > struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > struct drm_device *dev = dig_port->base.base.dev; > > @@ -666,9 +653,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, > > dev_priv->psr.psr2_enabled = crtc_state->has_psr2; > > dev_priv->psr.busy_frontbuffer_bits = 0; > > > > - dev_priv->psr.setup_vsc(intel_dp, crtc_state); > > - dev_priv->psr.enable_sink(intel_dp); > > - dev_priv->psr.enable_source(intel_dp, crtc_state); > > + intel_psr_setup_vsc(intel_dp, crtc_state); > > + intel_psr_enable_sink(intel_dp); > > + intel_psr_enable_source(intel_dp, crtc_state); > > dev_priv->psr.enabled = intel_dp; > > > > intel_psr_activate(intel_dp); > > @@ -677,8 +664,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, > > mutex_unlock(&dev_priv->psr.lock); > > } > > > > -static void hsw_psr_disable(struct intel_dp *intel_dp, > > - const struct intel_crtc_state *old_crtc_state) > > +static void > > +intel_psr_disable_source(struct intel_dp *intel_dp, > > + const struct intel_crtc_state *old_crtc_state) > > { > > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > struct drm_device *dev = intel_dig_port->base.base.dev; > > @@ -747,7 +735,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, > > return; > > } > > > > - dev_priv->psr.disable_source(intel_dp, old_crtc_state); > > + intel_psr_disable_source(intel_dp, old_crtc_state); > > > > /* Disable PSR on Sink */ > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0); > > @@ -986,11 +974,4 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > > > > INIT_WORK(&dev_priv->psr.work, intel_psr_work); > > mutex_init(&dev_priv->psr.lock); > > - > > - dev_priv->psr.enable_source = hsw_psr_enable_source; > > - dev_priv->psr.disable_source = hsw_psr_disable; > > - dev_priv->psr.enable_sink = hsw_psr_enable_sink; > > - dev_priv->psr.activate = hsw_psr_activate; > > - dev_priv->psr.setup_vsc = hsw_psr_setup_vsc; > > - > > } > > -- > > 2.17.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6f08ab310118..2b684f431c60 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -631,14 +631,6 @@ struct i915_psr { bool debug; ktime_t last_entry_attempt; ktime_t last_exit; - - void (*enable_source)(struct intel_dp *, - const struct intel_crtc_state *); - void (*disable_source)(struct intel_dp *, - const struct intel_crtc_state *); - void (*enable_sink)(struct intel_dp *); - void (*activate)(struct intel_dp *); - void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *); }; enum intel_pch { diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index aea81ace854b..bd8e562212f7 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,8 +278,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) } } -static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, - const struct intel_crtc_state *crtc_state) +static void intel_psr_setup_vsc(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); @@ -344,7 +344,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); } -static void hsw_psr_enable_sink(struct intel_dp *intel_dp) +static void intel_psr_enable_sink(struct intel_dp *intel_dp) { struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = dig_port->base.base.dev; @@ -456,24 +456,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) I915_WRITE(EDP_PSR2_CTL, val); } -static void hsw_psr_activate(struct intel_dp *intel_dp) -{ - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); - struct drm_device *dev = dig_port->base.base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - - /* On HSW+ after we enable PSR on source it will activate it - * as soon as it match configure idle_frame count. So - * we just actually enable it here on activation time. - */ - - /* psr1 and psr2 are mutually exclusive.*/ - if (dev_priv->psr.psr2_enabled) - hsw_activate_psr2(intel_dp); - else - hsw_activate_psr1(intel_dp); -} - static bool intel_psr2_config_valid(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state) { @@ -583,12 +565,17 @@ static void intel_psr_activate(struct intel_dp *intel_dp) WARN_ON(dev_priv->psr.active); lockdep_assert_held(&dev_priv->psr.lock); - dev_priv->psr.activate(intel_dp); + /* psr1 and psr2 are mutually exclusive.*/ + if (dev_priv->psr.psr2_enabled) + hsw_activate_psr2(intel_dp); + else + hsw_activate_psr1(intel_dp); + dev_priv->psr.active = true; } -static void hsw_psr_enable_source(struct intel_dp *intel_dp, - const struct intel_crtc_state *crtc_state) +static void intel_psr_enable_source(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) { struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = dig_port->base.base.dev; @@ -666,9 +653,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, dev_priv->psr.psr2_enabled = crtc_state->has_psr2; dev_priv->psr.busy_frontbuffer_bits = 0; - dev_priv->psr.setup_vsc(intel_dp, crtc_state); - dev_priv->psr.enable_sink(intel_dp); - dev_priv->psr.enable_source(intel_dp, crtc_state); + intel_psr_setup_vsc(intel_dp, crtc_state); + intel_psr_enable_sink(intel_dp); + intel_psr_enable_source(intel_dp, crtc_state); dev_priv->psr.enabled = intel_dp; intel_psr_activate(intel_dp); @@ -677,8 +664,9 @@ void intel_psr_enable(struct intel_dp *intel_dp, mutex_unlock(&dev_priv->psr.lock); } -static void hsw_psr_disable(struct intel_dp *intel_dp, - const struct intel_crtc_state *old_crtc_state) +static void +intel_psr_disable_source(struct intel_dp *intel_dp, + const struct intel_crtc_state *old_crtc_state) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port->base.base.dev; @@ -747,7 +735,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, return; } - dev_priv->psr.disable_source(intel_dp, old_crtc_state); + intel_psr_disable_source(intel_dp, old_crtc_state); /* Disable PSR on Sink */ drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0); @@ -986,11 +974,4 @@ void intel_psr_init(struct drm_i915_private *dev_priv) INIT_WORK(&dev_priv->psr.work, intel_psr_work); mutex_init(&dev_priv->psr.lock); - - dev_priv->psr.enable_source = hsw_psr_enable_source; - dev_priv->psr.disable_source = hsw_psr_disable; - dev_priv->psr.enable_sink = hsw_psr_enable_sink; - dev_priv->psr.activate = hsw_psr_activate; - dev_priv->psr.setup_vsc = hsw_psr_setup_vsc; - }
At some point we introduced the function pointers on PSR code to help with VLV/CHV separation logic because it had a different HW implementation from PSR. Since all converged to HSW PSR and we dropped the VLV/CHV support, let's also kill the useless function pointers and leave the code cleaner. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 8 ----- drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++--------------------- 2 files changed, 18 insertions(+), 45 deletions(-)