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 :/ >
