Re: [Intel-gfx] [PATCH 09/11] drm/i915: Add a platform independent way to check for CCS control planes

[Imre Deak <imre.deak@intel.com>]

On Thu, Oct 14, 2021 at 01:38:14AM +0300, Ville Syrjälä wrote:
> On Thu, Oct 14, 2021 at 01:28:24AM +0300, Imre Deak wrote:
> > On Thu, Oct 14, 2021 at 12:54:58AM +0300, Ville Syrjälä wrote:
> > > On Thu, Oct 14, 2021 at 12:32:55AM +0300, Imre Deak wrote:
> > > > On Wed, Oct 13, 2021 at 11:45:33PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Oct 13, 2021 at 11:27:02PM +0300, Ville Syrjälä wrote:
> > > > > > On Thu, Oct 07, 2021 at 11:35:15PM +0300, Imre Deak wrote:
> > > > > > > Future platforms change the location of CCS control planes in CCS
> > > > > > > framebuffers, so add intel_fb_is_rc_ccs_ctrl_plane() to query for these
> > > > > > 
> > > > > > Don't we use the term 'ccs_plane' everywhere else?
> > > > > > 
> > > > > > > planes independently of the platform. This function can be used
> > > > > > > everywhere instead of is_ccs_plane() (or is_ccs_plane() && !cc_plane()),
> > > > > > > since all the callers are only interested in control planes (and not CCS
> > > > > > > color-clear planes).
> > > > > 
> > > > > Hmm. I guess you're changing the terminology across the board?
> > > > > If it's used consistently then no objections from me.
> > > > 
> > > > ccs_plane has been used as a generic term for both the "control" and the
> > > > cc plane, or at least I thought of it as such.
> > > 
> > > The official definition I think is:
> > > CCS == color control surface
> > >
> > > So in terms of modifier naming I suppose I tend to think
> > > of it like this:
> > > modifier name has CCS -> color control surface is present
> > > modifier name has CC -> clear color is present
> > > 
> > > But if we want to make the distinction somehow stronger I was
> > > thinking maybe ccs_aux vs. ccs_cc. But dunno if that just ends up
> > > being more confusing since AUX_DIST is also used for planar scanout
> > > on skl/etc.
> 
> I guess the fact that it would also say "ccs" in additon to "aux"
> would make it ok. So ccs_aux goes into AUX_DIST, ccs_cc goes into CC_VAL.

Ok, ccs_aux works I guess, and it's actually used at a few places
already. So yes, not too consistent atm, will use ccs_aux in this
patchset and rename the remaining instances in a follow-up patch.

> But anyway, as long we go with something consitent everywhere I'll be
> happy.
> 
> -- 
> Ville Syrjälä
> Intel