Re: [PATCH v1 1/2] drm/i915/display: use PAGE_SIZE macro for FBC cfb alloc

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

On Thu, Jan 11, 2024 at 01:47:02PM +0000, Govindapillai, Vinod wrote:
> Hi Ville
> 
> The fix is in the next patch.
> 
> This pach changes the 4096 to page size macro as the BUG on is based on that macro explicitly.

I think the whole PAGE_SIZE handling should be in the xe code
since it's an implementation detail of the xe code.

> 
> Br
> Vinod
> 
> ________________________________
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Thursday, January 11, 2024 3:44:22 pm
> To: Govindapillai, Vinod <vinod.govindapillai@intel.com>
> Cc: intel-gfx@lists.freedesktop.org <intel-gfx@lists.freedesktop.org>; Syrjala, Ville <ville.syrjala@intel.com>
> Subject: Re: [PATCH v1 1/2] drm/i915/display: use PAGE_SIZE macro for FBC cfb alloc
> 
> On Wed, Jan 10, 2024 at 01:00:08PM +0200, Vinod Govindapillai wrote:
> > FBC compressed frame buffer size need to be PAGE_SIZE aligned
> > and the corresponding the drm_gem functions check the object
> > size alignment using PAGE_SIZE macro. Use the PAGE_SIZE macro
> > in the cfb alloc as well instead of the magic number.
> >
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index f17a1afb4929..9b9c8715d664 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -764,13 +764,15 @@ static int find_compression_limit(struct intel_fbc *fbc,
> >
> >        /* Try to over-allocate to reduce reallocations and fragmentation. */
> >        ret = i915_gem_stolen_insert_node_in_range(i915, &fbc->compressed_fb,
> > -                                                size <<= 1, 4096, 0, end);
> > +                                                size <<= 1, PAGE_SIZE, 0,
> > +                                                end);
> >        if (ret == 0)
> >                return limit;
> >
> >        for (; limit <= intel_fbc_max_limit(i915); limit <<= 1) {
> >                ret = i915_gem_stolen_insert_node_in_range(i915, &fbc->compressed_fb,
> > -                                                        size >>= 1, 4096, 0, end);
> > +                                                        size >>= 1, PAGE_SIZE, 0,
> > +                                                        end);
> 
> PAGE_SIZE is 4k so I can't see this doing anything at all.
> 
> The correct fix is probably either:
> - fix the xe gem code to always page align the size
> - page align it in xe's i915_gem_stolen_insert_node_in_range()
> 
> >                if (ret == 0)
> >                        return limit;
> >        }
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel
> 

-- 
Ville Syrjälä
Intel