Re: [PATCH 3/6] drm/i915/display: split out intel_fbc_regs.h from i915_reg.h

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 12, 2024 at 06:50:57PM +0300, Jani Nikula wrote:
> On Fri, 12 Apr 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Apr 12, 2024 at 05:52:55PM +0300, Jani Nikula wrote:
> >> +/*
> >> + * Framebuffer compression (915+ only)
> >> + */
> >
> > Outdated comment. Looks like pretty much all the comments
> > in this file are misleading/outdated. Maybe just nuke them
> > all while at it.
> 
> Ack.
> 
> >> +#define ILK_DISPLAY_CHICKEN1	_MMIO(0x42000)
> >
> > Not an FBC register.
> 
> Whoops, this one was an accident.
> 
> >> +#define CHICKEN_MISC_4		_MMIO(0x4208c)
> >
> > Also not an FBC register.
> 
> However this one was intentional. So the register isn't "an fbc
> register", but the contents are all about fbc,

Only the bits we have thus far defined. There's other stuff
in there that we haven't bothered to name.

> and it's only used in
> intel_fbc.c.

I don't think we should place chicken register/etc definitons based
purely on where it might be currently used. That may change at any
point when we discover a new chicken bit that needs to be flipped.
At that point the defintion would have to be moved again, or what
seems rather likely to happen, people will overlook the existing
definiton and add a duplicate elsewhere.

> 
> I guess after all reasonable topical things have been split out from
> i915_reg.h, whatever display stuff is left will need to be put to a new
> intel_display_regs.h or something. Things like this would then end up
> there. Better or worse that way, I don't know.

Yeah, there are a bunch of "these don't really belong anywhere"
registers. Though maybe this kind of non-specific chicken registers
could even live in the intel_chicken_regs.h file. Shrug.

-- 
Ville Syrjälä
Intel