[Intel-gfx] [PATCH 07/14] drm/i915: Clean up pre-skl primary plane registers

[Souza, Jose]

On Tue, 2022-01-18 at 02:55 +0200, Ville Syrjälä wrote:
> On Wed, Jan 12, 2022 at 08:12:31PM +0000, Souza, Jose wrote:
> > On Wed, 2021-12-01 at 17:25 +0200, Ville Syrjala wrote:
> > > @@ -427,9 +427,9 @@ static void i9xx_plane_update_noarm(struct intel_plane *plane,
> > >  		 * program whatever is there.
> > >  		 */
> > >  		intel_de_write_fw(dev_priv, DSPPOS(i9xx_plane),
> > > -				  (crtc_y << 16) | crtc_x);
> > > +				  DSP_POS_Y(crtc_y) | DSP_POS_X(crtc_x));
> > >  		intel_de_write_fw(dev_priv, DSPSIZE(i9xx_plane),
> > > -				  ((crtc_h - 1) << 16) | (crtc_w - 1));
> > > +				  DSP_HEIGHT(crtc_h - 1) | DSP_POS_X(crtc_w - 1));
> > 
> > DSP_HEIGHT(crtc_h - 1) | DSP_WIDTH(crtc_w - 1));
> 
> Whoops. Thanks for cathcing that.
> 
> <snip>
> > > +#define   DSP_ENABLE			REG_BIT(31)
> > 
> > I really don't like DSP, it is broadly used acronym to Digital Signal Processors.
> > Would prefer to have DISPLAY or DISP.
> 
> The registers are called DSP<foo>, so the spec makes the case for DSP_.
> The problem with DISP_/etc. is that the namespace then makes it a bit
> hard to figure out what register the defines belong to.
> 
> > 
> > Anyways, DSP_ENABLE should have also have plane on it.
> 
> DSP==display plane. Any more would be redundant.

Damn, even worst, thought it was DiSPlay.
But if this is the BSpec name, go ahead with it. 

> 
> > 
> > Other than above and a minor typo reported in general looks good to me but it also broke build because it missed GVT renames.
> 
> Always happens to me :/
>