On Thu, Jan 26, 2023 at 06:05:32PM +0200, Jani Nikula wrote:
On Thu, 26 Jan 2023, Luca Coelho <luca@xxxxxxxxx> wrote:
On Thu, 2023-01-26 at 14:11 +0200, Luca Coelho wrote:
On Thu, 2023-01-26 at 14:00 +0200, Jani Nikula wrote:
> On Thu, 26 Jan 2023, Luca Coelho <luca@xxxxxxxxx> 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).
Thanks for the clarification! In this case we're not using any of the
macros, AFAICT, so I guess it's better to go with i915 already? And I
think it should even be in this same patch, since it's a new function
anyway.
> 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.
I see that there is a boatload of register macros using it... I won't
promise, but I think it would be a good exercise for a n00b like me to
make this patch, though I already foresee another boatload of conflicts
with the internal trees and everything...
There were actually 10 boatloads of places to change:
187 files changed, 12104 insertions(+), 12104 deletions(-)
...but it _does_ compile. 😄
Do you think this is fine? Lots of shuffle, but if you think it's okay,
I can send the patch out now.
Heh, I said I'd take patchES, not everything together! ;)
Rodrigo, Tvrtko, Joonas, thoughts?
If it's a sed or something that can be automated, I think it could be
ok as single patch as long as we find the right time to generate it,
when the trees are in sync.
I do remember doing a sed s/dev_priv/i915/ (or it was with a cocci
script, don't remember) a few years ago, and I'm
glad we are giving up the slow conversion and just ripping the
bandaid.
Lucas De Marchi
