Re: [Intel-gfx] [PATCH 09/15] drm/i915: Define bitmasks for ilk pfit window pos/size

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

On Thu, Apr 20, 2023 at 03:09:11PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 19, 2023 at 06:34:00PM +0300, Jani Nikula wrote:
> > On Tue, 18 Apr 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Define and use the bitmasks for the x/y components
> > > of the ilk+ panel filter window pos/size registers.
> > 
> > This reduces the field sizes by 3-4 bits. Maybe that's what they're in
> > the spec, but it's at least worth mentioning here.
> 
> Aye. I just double checked this and on BDW these are in
> fact the only bits that can be set in the registers. On
> older hw every bit can apparently be set, but resumably
> the high bits just have no effect. And intel_mode_valid()
> will anyway reject modes with bigger hdisplay/vdisplay
> so we should never see out of bounds values here.

After pondering this a bit more, I think I'll go back to
16bit masks to make the thing more future proof. Seems unlikely
we'd get any other kinds of bits getting added to these registers
and we are using the full 16 bit masks also for the transcoder
timing registers and PIPESRC.

> 
> > 
> > BR,
> > Jani.
> > 
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++----
> > >  drivers/gpu/drm/i915/i915_reg.h              |  8 ++++++++
> > >  2 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index fb49d0ed61b4..626a5f41a1f1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -812,8 +812,10 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> > >  	else
> > >  		intel_de_write_fw(dev_priv, PF_CTL(pipe), PF_ENABLE |
> > >  				  PF_FILTER_MED_3x3);
> > > -	intel_de_write_fw(dev_priv, PF_WIN_POS(pipe), x << 16 | y);
> > > -	intel_de_write_fw(dev_priv, PF_WIN_SZ(pipe), width << 16 | height);
> > > +	intel_de_write_fw(dev_priv, PF_WIN_POS(pipe),
> > > +			  PF_WIN_XPOS(x) | PF_WIN_YPOS(y));
> > > +	intel_de_write_fw(dev_priv, PF_WIN_SZ(pipe),
> > > +			  PF_WIN_XSIZE(width) | PF_WIN_YSIZE(height));
> > >  }
> > >  
> > >  static void intel_crtc_dpms_overlay_disable(struct intel_crtc *crtc)
> > > @@ -3246,8 +3248,10 @@ static void ilk_get_pfit_config(struct intel_crtc_state *crtc_state)
> > >  	size = intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
> > >  
> > >  	drm_rect_init(&crtc_state->pch_pfit.dst,
> > > -		      pos >> 16, pos & 0xffff,
> > > -		      size >> 16, size & 0xffff);
> > > +		      REG_FIELD_GET(PF_WIN_XPOS_MASK, pos),
> > > +		      REG_FIELD_GET(PF_WIN_YPOS_MASK, pos),
> > > +		      REG_FIELD_GET(PF_WIN_XSIZE_MASK, size),
> > > +		      REG_FIELD_GET(PF_WIN_YSIZE_MASK, size));
> > >  
> > >  	/*
> > >  	 * We currently do not free assignements of panel fitters on
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 3c02f6c70733..75e1f30adda1 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4020,8 +4020,16 @@
> > >  #define   PF_FILTER_EDGE_SOFTEN		REG_FIELD_PREP(PF_FILTER_EDGE_MASK, 3)
> > >  #define _PFA_WIN_SZ		0x68074
> > >  #define _PFB_WIN_SZ		0x68874
> > > +#define   PF_WIN_XSIZE_MASK	REG_GENMASK(28, 16)
> > > +#define   PF_WIN_XSIZE(w)	REG_FIELD_PREP(PF_WIN_XSIZE_MASK, (w))
> > > +#define   PF_WIN_YSIZE_MASK	REG_GENMASK(11, 0)
> > > +#define   PF_WIN_YSIZE(h)	REG_FIELD_PREP(PF_WIN_YSIZE_MASK, (h))
> > >  #define _PFA_WIN_POS		0x68070
> > >  #define _PFB_WIN_POS		0x68870
> > > +#define   PF_WIN_XPOS_MASK	REG_GENMASK(28, 16)
> > > +#define   PF_WIN_XPOS(x)	REG_FIELD_PREP(PF_WIN_XPOS_MASK, (x))
> > > +#define   PF_WIN_YPOS_MASK	REG_GENMASK(11, 0)
> > > +#define   PF_WIN_YPOS(y)	REG_FIELD_PREP(PF_WIN_YPOS_MASK, (y))
> > >  #define _PFA_VSCALE		0x68084
> > >  #define _PFB_VSCALE		0x68884
> > >  #define _PFA_HSCALE		0x68090
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel