| Message ID | 20180314223617.20122-5-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2018-03-14 at 15:36 -0700, José Roberto de Souza wrote: > This value is a match of hardware and sink has PSR + if it can be > enabled by the requested state, see intel_psr_compute_config(). > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > drivers/gpu/drm/i915/intel_psr.c | 12 ++++++------ > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a215aa78b0be..cccaf84415ab 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -807,8 +807,8 @@ struct intel_crtc_state { > struct intel_link_m_n dp_m2_n2; > bool has_drrs; > > - bool has_psr; > - bool has_psr2; > + bool can_psr; > + bool can_psr2; I am not convinced by this change, the computed state either has PSR1 or PSR2, "can" connotes ambiguity in my opinion. I was thinking of converting this to an u8 psr = [0,1,2] to mean no PSR, PSR1 and PSR2 respectively. We can do away with the has/can confusion :) Using bool does save us 6 bits though depending on how the structure is packed.
On Thu, 2018-03-15 at 19:09 -0700, Pandiyan, Dhinakaran wrote: > > > On Wed, 2018-03-14 at 15:36 -0700, José Roberto de Souza wrote: > > This value is a match of hardware and sink has PSR + if it can be > > enabled by the requested state, see intel_psr_compute_config(). > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > > drivers/gpu/drm/i915/intel_psr.c | 12 ++++++------ > > 2 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index a215aa78b0be..cccaf84415ab 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -807,8 +807,8 @@ struct intel_crtc_state { > > struct intel_link_m_n dp_m2_n2; > > bool has_drrs; > > > > - bool has_psr; > > - bool has_psr2; > > + bool can_psr; > > + bool can_psr2; > > > I am not convinced by this change, the computed state either has PSR1 > or > PSR2, "can" connotes ambiguity in my opinion. Computed state can have PSR1 and PSR2 set. > > > I was thinking of converting this to an u8 psr = [0,1,2] to mean no > PSR, > PSR1 and PSR2 respectively. We can do away with the has/can confusion > :) > > Using bool does save us 6 bits though depending on how the structure > is > packed. I almost did this but I just renamed to do less changes. But cool I will rename it to psr and have 0, 1 or 2 values.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a215aa78b0be..cccaf84415ab 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -807,8 +807,8 @@ struct intel_crtc_state { struct intel_link_m_n dp_m2_n2; bool has_drrs; - bool has_psr; - bool has_psr2; + bool can_psr; + bool can_psr2; /* * Frequence the dpll for the port should run at. Differs from the diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 4cb613855c20..d622e37894d4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -560,9 +560,9 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, return; } - crtc_state->has_psr = true; - crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); - DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : ""); + crtc_state->can_psr = true; + crtc_state->can_psr2 = intel_psr2_config_valid(intel_dp, crtc_state); + DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->can_psr2 ? "2" : ""); } static void intel_psr_activate(struct intel_dp *intel_dp) @@ -632,7 +632,7 @@ void intel_psr_enable(struct intel_dp *intel_dp, struct drm_device *dev = intel_dig_port->base.base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc_state->has_psr) + if (!crtc_state->can_psr) return; if (WARN_ON(!CAN_PSR(dev_priv))) @@ -645,7 +645,7 @@ void intel_psr_enable(struct intel_dp *intel_dp, goto unlock; } - dev_priv->psr.psr2_enabled = crtc_state->has_psr2; + dev_priv->psr.psr2_enabled = crtc_state->can_psr2; dev_priv->psr.busy_frontbuffer_bits = 0; dev_priv->psr.setup_vsc(intel_dp, crtc_state); @@ -767,7 +767,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, struct drm_device *dev = intel_dig_port->base.base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!old_crtc_state->has_psr) + if (!old_crtc_state->can_psr) return; if (WARN_ON(!CAN_PSR(dev_priv)))
This value is a match of hardware and sink has PSR + if it can be enabled by the requested state, see intel_psr_compute_config(). Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/intel_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_psr.c | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-)