Re: [PATCH 5/6] drm/i915/psr: Rename intel_crtc_state has_psr to can_psr

Souza, Jose
2018-03-16T22:22:05Z

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.