Re: [Intel-gfx] [PATCH] drm/i915/psr: Split sel fetch plane configuration into arm and noarm

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 26 Jan 2023, Luca Coelho <luca@coelho.fi> wrote:
> On Wed, 2023-01-25 at 12:44 +0200, Jouni Högander wrote:
>> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>> > index 7d4a15a283a0..63b79c611932 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> > @@ -1559,7 +1559,26 @@ void intel_psr2_disable_plane_sel_fetch(struct intel_plane *plane,
>> >  	intel_de_write_fw(dev_priv, PLANE_SEL_FETCH_CTL(pipe, plane->id), 0);
>> >  }
>> >  
>> > -void intel_psr2_program_plane_sel_fetch(struct intel_plane *plane,
>> > +void intel_psr2_program_plane_sel_fetch_arm(struct intel_plane *plane,
>> > +					const struct intel_crtc_state *crtc_state,
>> > +					const struct intel_plane_state *plane_state,
>> > +					int color_plane)
>> > +{
>> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>
> Should you use i915 instead of dev_priv? I've heard and read elsewhere
> that this is generally a desired change.  Much easier to use always the
> same local name for this kind of thing.  Though this file is already
> interspersed with both versions...

Basically the only reason to use dev_priv for new code is to deal with
some register macros that still have implicit dev_priv in
them. Otherwise, i915 should be used, and when convenient, dev_priv
should be converted to i915 while touching the code anyway (in a
separate patch, but while you're there).

The implicit dev_priv dependencies in the register macros are a bit
annoying to fix, and it's been going slow. In retrospect maybe the right
thing would have been to just sed the parameter to all of them
everywhere and be done with it for good. Not too late now, I guess, and
I'd take the patches in a heartbeat if someone were to step up and do
it.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center