| Message ID | 20240705145254.3355-12-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/{i915, xe}: FBC cleanups + tweak fbdev stolen usage | expand |
> -----Original Message----- > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala > Sent: Friday, July 5, 2024 8:23 PM > To: intel-gfx@lists.freedesktop.org > Cc: intel-xe@lists.freedesktop.org > Subject: [PATCH 11/20] drm/i915/fbc: Extract intel_fbc_max_cfb_height() > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Pull the code to determine the maximum CFB height into a separate function. For > pre-HSW the maximum CFB height is the same as the maximum plane height (ie. > the older hardware supposedely doens't have the trick of leaving the extra lines > uncompressed). > Nit: Typo in supposedly and doesn't. Limit is also altered along with refactor; would a separate patch be better ? Leave it your judgement. Overall logic looks Good to me. Reviewed-by: Uma Shankar <uma.shankar@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 27 ++++++++++++++++++------ > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > b/drivers/gpu/drm/i915/display/intel_fbc.c > index cf5750ed4681..47b715e5d533 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -201,17 +201,30 @@ static unsigned int intel_fbc_cfb_stride(const struct > intel_plane_state *plane_s > return _intel_fbc_cfb_stride(display, width, stride); } > > -static unsigned int intel_fbc_cfb_size(const struct intel_plane_state > *plane_state) > +/* > + * Maximum height the hardware will compress, on HSW+ > + * additional lines (up to the actual plane height) will > + * remain uncompressed. > + */ > +static unsigned int intel_fbc_max_cfb_height(struct intel_display > +*display) > { > - struct intel_display *display = to_intel_display(plane_state->uapi.plane- > >dev); > - int height = drm_rect_height(&plane_state->uapi.src) >> 16; > + struct drm_i915_private *i915 = to_i915(display->drm); > > if (DISPLAY_VER(display) >= 8) > - height = min(height, 2560); > - else if (DISPLAY_VER(display) == 7) > - height = min(height, 2048); > + return 2560; > + else if (DISPLAY_VER(display) >= 5 || IS_G4X(i915)) > + return 2048; > + else > + return 1536; > +} > > - return height * intel_fbc_cfb_stride(plane_state); > +static unsigned int intel_fbc_cfb_size(const struct intel_plane_state > +*plane_state) { > + struct intel_display *display = to_intel_display(plane_state->uapi.plane- > >dev); > + unsigned int height = drm_rect_height(&plane_state->uapi.src) >> 16; > + > + return min(height, intel_fbc_max_cfb_height(display)) * > + intel_fbc_cfb_stride(plane_state); > } > > static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state > *plane_state) > -- > 2.44.2
On Wed, Jul 10, 2024 at 08:26:54AM +0000, Shankar, Uma wrote: > > > > -----Original Message----- > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala > > Sent: Friday, July 5, 2024 8:23 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: intel-xe@lists.freedesktop.org > > Subject: [PATCH 11/20] drm/i915/fbc: Extract intel_fbc_max_cfb_height() > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Pull the code to determine the maximum CFB height into a separate function. For > > pre-HSW the maximum CFB height is the same as the maximum plane height (ie. > > the older hardware supposedely doens't have the trick of leaving the extra lines > > uncompressed). > > > > Nit: Typo in supposedly and doesn't. > > Limit is also altered along with refactor; would a separate patch be better ? It doesn't actually change anything as that's just the max plane height for older platforms, so anything taller would have been already been rejected earlier. I suppose I could have added the explicit limit first, and then did the extraction. But seems harmless enough as is, so kept it like this but adjusted the commit message a bit to highlight this fact a little better. > Leave it your judgement. Overall logic looks Good to me. > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 27 ++++++++++++++++++------ > > 1 file changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index cf5750ed4681..47b715e5d533 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -201,17 +201,30 @@ static unsigned int intel_fbc_cfb_stride(const struct > > intel_plane_state *plane_s > > return _intel_fbc_cfb_stride(display, width, stride); } > > > > -static unsigned int intel_fbc_cfb_size(const struct intel_plane_state > > *plane_state) > > +/* > > + * Maximum height the hardware will compress, on HSW+ > > + * additional lines (up to the actual plane height) will > > + * remain uncompressed. > > + */ > > +static unsigned int intel_fbc_max_cfb_height(struct intel_display > > +*display) > > { > > - struct intel_display *display = to_intel_display(plane_state->uapi.plane- > > >dev); > > - int height = drm_rect_height(&plane_state->uapi.src) >> 16; > > + struct drm_i915_private *i915 = to_i915(display->drm); > > > > if (DISPLAY_VER(display) >= 8) > > - height = min(height, 2560); > > - else if (DISPLAY_VER(display) == 7) > > - height = min(height, 2048); > > + return 2560; > > + else if (DISPLAY_VER(display) >= 5 || IS_G4X(i915)) > > + return 2048; > > + else > > + return 1536; > > +} > > > > - return height * intel_fbc_cfb_stride(plane_state); > > +static unsigned int intel_fbc_cfb_size(const struct intel_plane_state > > +*plane_state) { > > + struct intel_display *display = to_intel_display(plane_state->uapi.plane- > > >dev); > > + unsigned int height = drm_rect_height(&plane_state->uapi.src) >> 16; > > + > > + return min(height, intel_fbc_max_cfb_height(display)) * > > + intel_fbc_cfb_stride(plane_state); > > } > > > > static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state > > *plane_state) > > -- > > 2.44.2 >
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index cf5750ed4681..47b715e5d533 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -201,17 +201,30 @@ static unsigned int intel_fbc_cfb_stride(const struct intel_plane_state *plane_s return _intel_fbc_cfb_stride(display, width, stride); } -static unsigned int intel_fbc_cfb_size(const struct intel_plane_state *plane_state) +/* + * Maximum height the hardware will compress, on HSW+ + * additional lines (up to the actual plane height) will + * remain uncompressed. + */ +static unsigned int intel_fbc_max_cfb_height(struct intel_display *display) { - struct intel_display *display = to_intel_display(plane_state->uapi.plane->dev); - int height = drm_rect_height(&plane_state->uapi.src) >> 16; + struct drm_i915_private *i915 = to_i915(display->drm); if (DISPLAY_VER(display) >= 8) - height = min(height, 2560); - else if (DISPLAY_VER(display) == 7) - height = min(height, 2048); + return 2560; + else if (DISPLAY_VER(display) >= 5 || IS_G4X(i915)) + return 2048; + else + return 1536; +} - return height * intel_fbc_cfb_stride(plane_state); +static unsigned int intel_fbc_cfb_size(const struct intel_plane_state *plane_state) +{ + struct intel_display *display = to_intel_display(plane_state->uapi.plane->dev); + unsigned int height = drm_rect_height(&plane_state->uapi.src) >> 16; + + return min(height, intel_fbc_max_cfb_height(display)) * + intel_fbc_cfb_stride(plane_state); } static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state *plane_state)