diff mbox series

[v2,1/3] drm/i915: Move stolen memory handling into i915_gem_stolen

Message ID 20230614051731.745821-2-jouni.hogander@intel.com (mailing list archive)
State New, archived
Headers show
Series Move stolen memory handling details into i915_gem_stolen | expand

Commit Message

Hogander, Jouni June 14, 2023, 5:17 a.m. UTC
We are preparing for Xe. Xe stolen memory handling differs from i915 so we
want to move stolen memory handling details into i915_gem_stolen.

Also add a common type for fbc compressed fb and use it from fbc code
instead of underlying type directly. This way we can have common type
i915_stolen_fb for both i915 and Xe.

v2: Fix couple of checkpatch warnings

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++-----------
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36 +++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
 3 files changed, 73 insertions(+), 22 deletions(-)

Comments

Nirmoy Das Aug. 1, 2023, 8:02 a.m. UTC | #1
Hi Jouni,

On 6/14/2023 7:17 AM, Jouni Högander wrote:
> We are preparing for Xe. Xe stolen memory handling differs from i915 so we
> want to move stolen memory handling details into i915_gem_stolen.
>
> Also add a common type for fbc compressed fb and use it from fbc code
> instead of underlying type directly. This way we can have common type
> i915_stolen_fb for both i915 and Xe.
>
> v2: Fix couple of checkpatch warnings
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++-----------
>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36 +++++++++++++++++
>   drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
>   3 files changed, 73 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 7f8b2d7713c7..a18e84efe911 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -94,8 +94,7 @@ struct intel_fbc {
>   	struct mutex lock;
>   	unsigned int busy_bits;
>   
> -	struct drm_mm_node compressed_fb;
> -	struct drm_mm_node compressed_llb;
> +	struct i915_stolen_fb compressed_fb, compressed_llb;
>   
>   	enum intel_fbc_id id;
>   
> @@ -332,15 +331,16 @@ static void i8xx_fbc_program_cfb(struct intel_fbc *fbc)
>   {
>   	struct drm_i915_private *i915 = fbc->i915;
>   
> -	GEM_BUG_ON(range_overflows_end_t(u64, i915->dsm.stolen.start,
> -					 fbc->compressed_fb.start, U32_MAX));
> -	GEM_BUG_ON(range_overflows_end_t(u64, i915->dsm.stolen.start,
> -					 fbc->compressed_llb.start, U32_MAX));
> -
> +	GEM_BUG_ON(range_overflows_end_t(u64, i915_gem_stolen_area_address(i915),
> +					 i915_gem_stolen_node_offset(&fbc->compressed_fb),
> +					 U32_MAX));
> +	GEM_BUG_ON(range_overflows_end_t(u64, i915_gem_stolen_area_address(i915),
> +					 i915_gem_stolen_node_offset(&fbc->compressed_llb),
> +					 U32_MAX));
>   	intel_de_write(i915, FBC_CFB_BASE,
> -		       i915->dsm.stolen.start + fbc->compressed_fb.start);
> +		       i915_gem_stolen_node_address(i915, &fbc->compressed_fb));
>   	intel_de_write(i915, FBC_LL_BASE,
> -		       i915->dsm.stolen.start + fbc->compressed_llb.start);
> +		       i915_gem_stolen_node_address(i915, &fbc->compressed_llb));
>   }
>   
>   static const struct intel_fbc_funcs i8xx_fbc_funcs = {
> @@ -447,7 +447,8 @@ static void g4x_fbc_program_cfb(struct intel_fbc *fbc)
>   {
>   	struct drm_i915_private *i915 = fbc->i915;
>   
> -	intel_de_write(i915, DPFC_CB_BASE, fbc->compressed_fb.start);
> +	intel_de_write(i915, DPFC_CB_BASE,
> +		       i915_gem_stolen_node_offset(&fbc->compressed_fb));
>   }
>   
>   static const struct intel_fbc_funcs g4x_fbc_funcs = {
> @@ -498,7 +499,8 @@ static void ilk_fbc_program_cfb(struct intel_fbc *fbc)
>   {
>   	struct drm_i915_private *i915 = fbc->i915;
>   
> -	intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc->compressed_fb.start);
> +	intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
> +		       i915_gem_stolen_node_offset(&fbc->compressed_fb));
>   }
>   
>   static const struct intel_fbc_funcs ilk_fbc_funcs = {
> @@ -713,7 +715,7 @@ static u64 intel_fbc_stolen_end(struct drm_i915_private *i915)
>   	 * underruns, even if that range is not reserved by the BIOS. */
>   	if (IS_BROADWELL(i915) ||
>   	    (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
> -		end = resource_size(&i915->dsm.stolen) - 8 * 1024 * 1024;
> +		end = i915_gem_stolen_area_size(i915) - 8 * 1024 * 1024;
>   	else
>   		end = U64_MAX;
>   
> @@ -770,9 +772,9 @@ static int intel_fbc_alloc_cfb(struct intel_fbc *fbc,
>   	int ret;
>   
>   	drm_WARN_ON(&i915->drm,
> -		    drm_mm_node_allocated(&fbc->compressed_fb));
> +		    i915_gem_stolen_node_allocated(&fbc->compressed_fb));
>   	drm_WARN_ON(&i915->drm,
> -		    drm_mm_node_allocated(&fbc->compressed_llb));
> +		    i915_gem_stolen_node_allocated(&fbc->compressed_llb));
>   
>   	if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
>   		ret = i915_gem_stolen_insert_node(i915, &fbc->compressed_llb,
> @@ -792,15 +794,14 @@ static int intel_fbc_alloc_cfb(struct intel_fbc *fbc,
>   
>   	drm_dbg_kms(&i915->drm,
>   		    "reserved %llu bytes of contiguous stolen space for FBC, limit: %d\n",
> -		    fbc->compressed_fb.size, fbc->limit);
> -
> +		    i915_gem_stolen_node_size(&fbc->compressed_fb), fbc->limit);
>   	return 0;
>   
>   err_llb:
> -	if (drm_mm_node_allocated(&fbc->compressed_llb))
> +	if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
>   		i915_gem_stolen_remove_node(i915, &fbc->compressed_llb);
>   err:
> -	if (drm_mm_initialized(&i915->mm.stolen))
> +	if (i915_gem_stolen_initialized(i915))
>   		drm_info_once(&i915->drm, "not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
>   	return -ENOSPC;
>   }
> @@ -825,9 +826,9 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
>   	if (WARN_ON(intel_fbc_hw_is_active(fbc)))
>   		return;
>   
> -	if (drm_mm_node_allocated(&fbc->compressed_llb))
> +	if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
>   		i915_gem_stolen_remove_node(i915, &fbc->compressed_llb);
> -	if (drm_mm_node_allocated(&fbc->compressed_fb))
> +	if (i915_gem_stolen_node_allocated(&fbc->compressed_fb))
>   		i915_gem_stolen_remove_node(i915, &fbc->compressed_fb);
>   }
>   
> @@ -1030,7 +1031,8 @@ static bool intel_fbc_is_cfb_ok(const struct intel_plane_state *plane_state)
>   	struct intel_fbc *fbc = plane->fbc;
>   
>   	return intel_fbc_min_limit(plane_state) <= fbc->limit &&
> -		intel_fbc_cfb_size(plane_state) <= fbc->compressed_fb.size * fbc->limit;
> +		intel_fbc_cfb_size(plane_state) <= fbc->limit *
> +			i915_gem_stolen_node_size(&fbc->compressed_fb);
>   }
>   
>   static bool intel_fbc_is_ok(const struct intel_plane_state *plane_state)
> @@ -1707,7 +1709,7 @@ void intel_fbc_init(struct drm_i915_private *i915)
>   {
>   	enum intel_fbc_id fbc_id;
>   
> -	if (!drm_mm_initialized(&i915->mm.stolen))
> +	if (!i915_gem_stolen_initialized(i915))
>   		DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
>   
>   	if (need_fbc_vtd_wa(i915))
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 3b094d36a0b0..78bac1e611dd 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -974,3 +974,39 @@ bool i915_gem_object_is_stolen(const struct drm_i915_gem_object *obj)
>   {
>   	return obj->ops == &i915_gem_object_stolen_ops;
>   }
> +
> +bool i915_gem_stolen_initialized(const struct drm_i915_private *i915)
> +{
> +	return drm_mm_initialized(&i915->mm.stolen);
> +}
> +
> +u64 i915_gem_stolen_area_address(const struct drm_i915_private *i915)
> +{
> +	return i915->dsm.stolen.start;
> +}
> +
> +u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915)
> +{
> +	return resource_size(&i915->dsm.stolen);
> +}
> +
> +u64 i915_gem_stolen_node_address(const struct drm_i915_private *i915,
> +				 const struct drm_mm_node *node)
> +{
> +	return i915->dsm.stolen.start + i915_gem_stolen_node_offset(node);
> +}
> +
> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node *node)
> +{
> +	return drm_mm_node_allocated(node);
> +}
> +
> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node)
> +{
> +	return node->start;
> +}
> +
> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
> +{
> +	return node->size;


Above 3 functions are core drm functions/struct and not related to 
stolen so I don't think

they deserve special functions in stolen code.


Regards,

Nirmoy

> +}
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> index d5005a39d130..258381d1c054 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> @@ -12,6 +12,8 @@ struct drm_i915_private;
>   struct drm_mm_node;
>   struct drm_i915_gem_object;
>   
> +#define i915_stolen_fb drm_mm_node
> +
>   int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>   				struct drm_mm_node *node, u64 size,
>   				unsigned alignment);
> @@ -36,4 +38,15 @@ bool i915_gem_object_is_stolen(const struct drm_i915_gem_object *obj);
>   
>   #define I915_GEM_STOLEN_BIAS SZ_128K
>   
> +bool i915_gem_stolen_initialized(const struct drm_i915_private *i915);
> +u64 i915_gem_stolen_area_address(const struct drm_i915_private *i915);
> +u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915);
> +
> +u64 i915_gem_stolen_node_address(const struct drm_i915_private *i915,
> +				 const struct drm_mm_node *node);
> +
> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node *node);
> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node);
> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
> +
>   #endif /* __I915_GEM_STOLEN_H__ */
Hogander, Jouni Aug. 1, 2023, 8:33 a.m. UTC | #2
On Tue, 2023-08-01 at 10:02 +0200, Nirmoy Das wrote:
> Hi Jouni,
> 
> On 6/14/2023 7:17 AM, Jouni Högander wrote:
> > We are preparing for Xe. Xe stolen memory handling differs from
> > i915 so we
> > want to move stolen memory handling details into i915_gem_stolen.
> > 
> > Also add a common type for fbc compressed fb and use it from fbc
> > code
> > instead of underlying type directly. This way we can have common
> > type
> > i915_stolen_fb for both i915 and Xe.
> > 
> > v2: Fix couple of checkpatch warnings
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > Signed-off-by: Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++------
> > -----
> >   drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36 +++++++++++++++++
> >   drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
> >   3 files changed, 73 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 7f8b2d7713c7..a18e84efe911 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -94,8 +94,7 @@ struct intel_fbc {
> >         struct mutex lock;
> >         unsigned int busy_bits;
> >   
> > -       struct drm_mm_node compressed_fb;
> > -       struct drm_mm_node compressed_llb;
> > +       struct i915_stolen_fb compressed_fb, compressed_llb;
> >   
> >         enum intel_fbc_id id;
> >   
> > @@ -332,15 +331,16 @@ static void i8xx_fbc_program_cfb(struct
> > intel_fbc *fbc)
> >   {
> >         struct drm_i915_private *i915 = fbc->i915;
> >   
> > -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
> > >dsm.stolen.start,
> > -                                        fbc->compressed_fb.start,
> > U32_MAX));
> > -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
> > >dsm.stolen.start,
> > -                                        fbc->compressed_llb.start,
> > U32_MAX));
> > -
> > +       GEM_BUG_ON(range_overflows_end_t(u64,
> > i915_gem_stolen_area_address(i915),
> > +                                       
> > i915_gem_stolen_node_offset(&fbc->compressed_fb),
> > +                                        U32_MAX));
> > +       GEM_BUG_ON(range_overflows_end_t(u64,
> > i915_gem_stolen_area_address(i915),
> > +                                       
> > i915_gem_stolen_node_offset(&fbc->compressed_llb),
> > +                                        U32_MAX));
> >         intel_de_write(i915, FBC_CFB_BASE,
> > -                      i915->dsm.stolen.start + fbc-
> > >compressed_fb.start);
> > +                      i915_gem_stolen_node_address(i915, &fbc-
> > >compressed_fb));
> >         intel_de_write(i915, FBC_LL_BASE,
> > -                      i915->dsm.stolen.start + fbc-
> > >compressed_llb.start);
> > +                      i915_gem_stolen_node_address(i915, &fbc-
> > >compressed_llb));
> >   }
> >   
> >   static const struct intel_fbc_funcs i8xx_fbc_funcs = {
> > @@ -447,7 +447,8 @@ static void g4x_fbc_program_cfb(struct
> > intel_fbc *fbc)
> >   {
> >         struct drm_i915_private *i915 = fbc->i915;
> >   
> > -       intel_de_write(i915, DPFC_CB_BASE, fbc-
> > >compressed_fb.start);
> > +       intel_de_write(i915, DPFC_CB_BASE,
> > +                      i915_gem_stolen_node_offset(&fbc-
> > >compressed_fb));
> >   }
> >   
> >   static const struct intel_fbc_funcs g4x_fbc_funcs = {
> > @@ -498,7 +499,8 @@ static void ilk_fbc_program_cfb(struct
> > intel_fbc *fbc)
> >   {
> >         struct drm_i915_private *i915 = fbc->i915;
> >   
> > -       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc-
> > >compressed_fb.start);
> > +       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
> > +                      i915_gem_stolen_node_offset(&fbc-
> > >compressed_fb));
> >   }
> >   
> >   static const struct intel_fbc_funcs ilk_fbc_funcs = {
> > @@ -713,7 +715,7 @@ static u64 intel_fbc_stolen_end(struct
> > drm_i915_private *i915)
> >          * underruns, even if that range is not reserved by the
> > BIOS. */
> >         if (IS_BROADWELL(i915) ||
> >             (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
> > -               end = resource_size(&i915->dsm.stolen) - 8 * 1024 *
> > 1024;
> > +               end = i915_gem_stolen_area_size(i915) - 8 * 1024 *
> > 1024;
> >         else
> >                 end = U64_MAX;
> >   
> > @@ -770,9 +772,9 @@ static int intel_fbc_alloc_cfb(struct intel_fbc
> > *fbc,
> >         int ret;
> >   
> >         drm_WARN_ON(&i915->drm,
> > -                   drm_mm_node_allocated(&fbc->compressed_fb));
> > +                   i915_gem_stolen_node_allocated(&fbc-
> > >compressed_fb));
> >         drm_WARN_ON(&i915->drm,
> > -                   drm_mm_node_allocated(&fbc->compressed_llb));
> > +                   i915_gem_stolen_node_allocated(&fbc-
> > >compressed_llb));
> >   
> >         if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
> >                 ret = i915_gem_stolen_insert_node(i915, &fbc-
> > >compressed_llb,
> > @@ -792,15 +794,14 @@ static int intel_fbc_alloc_cfb(struct
> > intel_fbc *fbc,
> >   
> >         drm_dbg_kms(&i915->drm,
> >                     "reserved %llu bytes of contiguous stolen space
> > for FBC, limit: %d\n",
> > -                   fbc->compressed_fb.size, fbc->limit);
> > -
> > +                   i915_gem_stolen_node_size(&fbc->compressed_fb),
> > fbc->limit);
> >         return 0;
> >   
> >   err_llb:
> > -       if (drm_mm_node_allocated(&fbc->compressed_llb))
> > +       if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
> >                 i915_gem_stolen_remove_node(i915, &fbc-
> > >compressed_llb);
> >   err:
> > -       if (drm_mm_initialized(&i915->mm.stolen))
> > +       if (i915_gem_stolen_initialized(i915))
> >                 drm_info_once(&i915->drm, "not enough stolen space
> > for compressed buffer (need %d more bytes), disabling. Hint: you
> > may be able to increase stolen memory size in the BIOS to avoid
> > this.\n", size);
> >         return -ENOSPC;
> >   }
> > @@ -825,9 +826,9 @@ static void __intel_fbc_cleanup_cfb(struct
> > intel_fbc *fbc)
> >         if (WARN_ON(intel_fbc_hw_is_active(fbc)))
> >                 return;
> >   
> > -       if (drm_mm_node_allocated(&fbc->compressed_llb))
> > +       if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
> >                 i915_gem_stolen_remove_node(i915, &fbc-
> > >compressed_llb);
> > -       if (drm_mm_node_allocated(&fbc->compressed_fb))
> > +       if (i915_gem_stolen_node_allocated(&fbc->compressed_fb))
> >                 i915_gem_stolen_remove_node(i915, &fbc-
> > >compressed_fb);
> >   }
> >   
> > @@ -1030,7 +1031,8 @@ static bool intel_fbc_is_cfb_ok(const struct
> > intel_plane_state *plane_state)
> >         struct intel_fbc *fbc = plane->fbc;
> >   
> >         return intel_fbc_min_limit(plane_state) <= fbc->limit &&
> > -               intel_fbc_cfb_size(plane_state) <= fbc-
> > >compressed_fb.size * fbc->limit;
> > +               intel_fbc_cfb_size(plane_state) <= fbc->limit *
> > +                       i915_gem_stolen_node_size(&fbc-
> > >compressed_fb);
> >   }
> >   
> >   static bool intel_fbc_is_ok(const struct intel_plane_state
> > *plane_state)
> > @@ -1707,7 +1709,7 @@ void intel_fbc_init(struct drm_i915_private
> > *i915)
> >   {
> >         enum intel_fbc_id fbc_id;
> >   
> > -       if (!drm_mm_initialized(&i915->mm.stolen))
> > +       if (!i915_gem_stolen_initialized(i915))
> >                 DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
> >   
> >         if (need_fbc_vtd_wa(i915))
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > index 3b094d36a0b0..78bac1e611dd 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > @@ -974,3 +974,39 @@ bool i915_gem_object_is_stolen(const struct
> > drm_i915_gem_object *obj)
> >   {
> >         return obj->ops == &i915_gem_object_stolen_ops;
> >   }
> > +
> > +bool i915_gem_stolen_initialized(const struct drm_i915_private
> > *i915)
> > +{
> > +       return drm_mm_initialized(&i915->mm.stolen);
> > +}
> > +
> > +u64 i915_gem_stolen_area_address(const struct drm_i915_private
> > *i915)
> > +{
> > +       return i915->dsm.stolen.start;
> > +}
> > +
> > +u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915)
> > +{
> > +       return resource_size(&i915->dsm.stolen);
> > +}
> > +
> > +u64 i915_gem_stolen_node_address(const struct drm_i915_private
> > *i915,
> > +                                const struct drm_mm_node *node)
> > +{
> > +       return i915->dsm.stolen.start +
> > i915_gem_stolen_node_offset(node);
> > +}
> > +
> > +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
> > *node)
> > +{
> > +       return drm_mm_node_allocated(node);
> > +}
> > +
> > +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node)
> > +{
> > +       return node->start;
> > +}
> > +
> > +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
> > +{
> > +       return node->size;
> 
> 
> Above 3 functions are core drm functions/struct and not related to 
> stolen so I don't think
> 
> they deserve special functions in stolen code.

Xe and i915 have differing implementations for stolen memory. We want
to remove these details from FBC code. The thing here is that in i915
case stolen memory node == drm mm node. In Xe case it is not and
interfaces for these queries will have own implementation for Xe. See:

https://patchwork.freedesktop.org/patch/540793/?series=118560&rev=3

BR,

Jouni Högander
> 
> 
> Regards,
> 
> Nirmoy
> 
> > +}
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > index d5005a39d130..258381d1c054 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > @@ -12,6 +12,8 @@ struct drm_i915_private;
> >   struct drm_mm_node;
> >   struct drm_i915_gem_object;
> >   
> > +#define i915_stolen_fb drm_mm_node
> > +
> >   int i915_gem_stolen_insert_node(struct drm_i915_private
> > *dev_priv,
> >                                 struct drm_mm_node *node, u64 size,
> >                                 unsigned alignment);
> > @@ -36,4 +38,15 @@ bool i915_gem_object_is_stolen(const struct
> > drm_i915_gem_object *obj);
> >   
> >   #define I915_GEM_STOLEN_BIAS SZ_128K
> >   
> > +bool i915_gem_stolen_initialized(const struct drm_i915_private
> > *i915);
> > +u64 i915_gem_stolen_area_address(const struct drm_i915_private
> > *i915);
> > +u64 i915_gem_stolen_area_size(const struct drm_i915_private
> > *i915);
> > +
> > +u64 i915_gem_stolen_node_address(const struct drm_i915_private
> > *i915,
> > +                                const struct drm_mm_node *node);
> > +
> > +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
> > *node);
> > +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node);
> > +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
> > +
> >   #endif /* __I915_GEM_STOLEN_H__ */
Nirmoy Das Aug. 2, 2023, 7:51 a.m. UTC | #3
On 8/1/2023 10:33 AM, Hogander, Jouni wrote:
> On Tue, 2023-08-01 at 10:02 +0200, Nirmoy Das wrote:
>> Hi Jouni,
>>
>> On 6/14/2023 7:17 AM, Jouni Högander wrote:
>>> We are preparing for Xe. Xe stolen memory handling differs from
>>> i915 so we
>>> want to move stolen memory handling details into i915_gem_stolen.
>>>
>>> Also add a common type for fbc compressed fb and use it from fbc
>>> code
>>> instead of underlying type directly. This way we can have common
>>> type
>>> i915_stolen_fb for both i915 and Xe.
>>>
>>> v2: Fix couple of checkpatch warnings
>>>
>>> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>>> Signed-off-by: Maarten Lankhorst
>>> <maarten.lankhorst@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++------
>>> -----
>>>    drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36 +++++++++++++++++
>>>    drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
>>>    3 files changed, 73 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
>>> b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> index 7f8b2d7713c7..a18e84efe911 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> @@ -94,8 +94,7 @@ struct intel_fbc {
>>>          struct mutex lock;
>>>          unsigned int busy_bits;
>>>    
>>> -       struct drm_mm_node compressed_fb;
>>> -       struct drm_mm_node compressed_llb;
>>> +       struct i915_stolen_fb compressed_fb, compressed_llb;
>>>    
>>>          enum intel_fbc_id id;
>>>    
>>> @@ -332,15 +331,16 @@ static void i8xx_fbc_program_cfb(struct
>>> intel_fbc *fbc)
>>>    {
>>>          struct drm_i915_private *i915 = fbc->i915;
>>>    
>>> -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
>>>> dsm.stolen.start,
>>> -                                        fbc->compressed_fb.start,
>>> U32_MAX));
>>> -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
>>>> dsm.stolen.start,
>>> -                                        fbc->compressed_llb.start,
>>> U32_MAX));
>>> -
>>> +       GEM_BUG_ON(range_overflows_end_t(u64,
>>> i915_gem_stolen_area_address(i915),
>>> +
>>> i915_gem_stolen_node_offset(&fbc->compressed_fb),
>>> +                                        U32_MAX));
>>> +       GEM_BUG_ON(range_overflows_end_t(u64,
>>> i915_gem_stolen_area_address(i915),
>>> +
>>> i915_gem_stolen_node_offset(&fbc->compressed_llb),
>>> +                                        U32_MAX));
>>>          intel_de_write(i915, FBC_CFB_BASE,
>>> -                      i915->dsm.stolen.start + fbc-
>>>> compressed_fb.start);
>>> +                      i915_gem_stolen_node_address(i915, &fbc-
>>>> compressed_fb));
>>>          intel_de_write(i915, FBC_LL_BASE,
>>> -                      i915->dsm.stolen.start + fbc-
>>>> compressed_llb.start);
>>> +                      i915_gem_stolen_node_address(i915, &fbc-
>>>> compressed_llb));
>>>    }
>>>    
>>>    static const struct intel_fbc_funcs i8xx_fbc_funcs = {
>>> @@ -447,7 +447,8 @@ static void g4x_fbc_program_cfb(struct
>>> intel_fbc *fbc)
>>>    {
>>>          struct drm_i915_private *i915 = fbc->i915;
>>>    
>>> -       intel_de_write(i915, DPFC_CB_BASE, fbc-
>>>> compressed_fb.start);
>>> +       intel_de_write(i915, DPFC_CB_BASE,
>>> +                      i915_gem_stolen_node_offset(&fbc-
>>>> compressed_fb));
>>>    }
>>>    
>>>    static const struct intel_fbc_funcs g4x_fbc_funcs = {
>>> @@ -498,7 +499,8 @@ static void ilk_fbc_program_cfb(struct
>>> intel_fbc *fbc)
>>>    {
>>>          struct drm_i915_private *i915 = fbc->i915;
>>>    
>>> -       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc-
>>>> compressed_fb.start);
>>> +       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
>>> +                      i915_gem_stolen_node_offset(&fbc-
>>>> compressed_fb));
>>>    }
>>>    
>>>    static const struct intel_fbc_funcs ilk_fbc_funcs = {
>>> @@ -713,7 +715,7 @@ static u64 intel_fbc_stolen_end(struct
>>> drm_i915_private *i915)
>>>           * underruns, even if that range is not reserved by the
>>> BIOS. */
>>>          if (IS_BROADWELL(i915) ||
>>>              (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
>>> -               end = resource_size(&i915->dsm.stolen) - 8 * 1024 *
>>> 1024;
>>> +               end = i915_gem_stolen_area_size(i915) - 8 * 1024 *
>>> 1024;
>>>          else
>>>                  end = U64_MAX;
>>>    
>>> @@ -770,9 +772,9 @@ static int intel_fbc_alloc_cfb(struct intel_fbc
>>> *fbc,
>>>          int ret;
>>>    
>>>          drm_WARN_ON(&i915->drm,
>>> -                   drm_mm_node_allocated(&fbc->compressed_fb));
>>> +                   i915_gem_stolen_node_allocated(&fbc-
>>>> compressed_fb));
>>>          drm_WARN_ON(&i915->drm,
>>> -                   drm_mm_node_allocated(&fbc->compressed_llb));
>>> +                   i915_gem_stolen_node_allocated(&fbc-
>>>> compressed_llb));
>>>    
>>>          if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
>>>                  ret = i915_gem_stolen_insert_node(i915, &fbc-
>>>> compressed_llb,
>>> @@ -792,15 +794,14 @@ static int intel_fbc_alloc_cfb(struct
>>> intel_fbc *fbc,
>>>    
>>>          drm_dbg_kms(&i915->drm,
>>>                      "reserved %llu bytes of contiguous stolen space
>>> for FBC, limit: %d\n",
>>> -                   fbc->compressed_fb.size, fbc->limit);
>>> -
>>> +                   i915_gem_stolen_node_size(&fbc->compressed_fb),
>>> fbc->limit);
>>>          return 0;
>>>    
>>>    err_llb:
>>> -       if (drm_mm_node_allocated(&fbc->compressed_llb))
>>> +       if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
>>>                  i915_gem_stolen_remove_node(i915, &fbc-
>>>> compressed_llb);
>>>    err:
>>> -       if (drm_mm_initialized(&i915->mm.stolen))
>>> +       if (i915_gem_stolen_initialized(i915))
>>>                  drm_info_once(&i915->drm, "not enough stolen space
>>> for compressed buffer (need %d more bytes), disabling. Hint: you
>>> may be able to increase stolen memory size in the BIOS to avoid
>>> this.\n", size);
>>>          return -ENOSPC;
>>>    }
>>> @@ -825,9 +826,9 @@ static void __intel_fbc_cleanup_cfb(struct
>>> intel_fbc *fbc)
>>>          if (WARN_ON(intel_fbc_hw_is_active(fbc)))
>>>                  return;
>>>    
>>> -       if (drm_mm_node_allocated(&fbc->compressed_llb))
>>> +       if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
>>>                  i915_gem_stolen_remove_node(i915, &fbc-
>>>> compressed_llb);
>>> -       if (drm_mm_node_allocated(&fbc->compressed_fb))
>>> +       if (i915_gem_stolen_node_allocated(&fbc->compressed_fb))
>>>                  i915_gem_stolen_remove_node(i915, &fbc-
>>>> compressed_fb);
>>>    }
>>>    
>>> @@ -1030,7 +1031,8 @@ static bool intel_fbc_is_cfb_ok(const struct
>>> intel_plane_state *plane_state)
>>>          struct intel_fbc *fbc = plane->fbc;
>>>    
>>>          return intel_fbc_min_limit(plane_state) <= fbc->limit &&
>>> -               intel_fbc_cfb_size(plane_state) <= fbc-
>>>> compressed_fb.size * fbc->limit;
>>> +               intel_fbc_cfb_size(plane_state) <= fbc->limit *
>>> +                       i915_gem_stolen_node_size(&fbc-
>>>> compressed_fb);
>>>    }
>>>    
>>>    static bool intel_fbc_is_ok(const struct intel_plane_state
>>> *plane_state)
>>> @@ -1707,7 +1709,7 @@ void intel_fbc_init(struct drm_i915_private
>>> *i915)
>>>    {
>>>          enum intel_fbc_id fbc_id;
>>>    
>>> -       if (!drm_mm_initialized(&i915->mm.stolen))
>>> +       if (!i915_gem_stolen_initialized(i915))
>>>                  DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
>>>    
>>>          if (need_fbc_vtd_wa(i915))
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> index 3b094d36a0b0..78bac1e611dd 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> @@ -974,3 +974,39 @@ bool i915_gem_object_is_stolen(const struct
>>> drm_i915_gem_object *obj)
>>>    {
>>>          return obj->ops == &i915_gem_object_stolen_ops;
>>>    }
>>> +
>>> +bool i915_gem_stolen_initialized(const struct drm_i915_private
>>> *i915)
>>> +{
>>> +       return drm_mm_initialized(&i915->mm.stolen);
>>> +}
>>> +
>>> +u64 i915_gem_stolen_area_address(const struct drm_i915_private
>>> *i915)
>>> +{
>>> +       return i915->dsm.stolen.start;
>>> +}
>>> +
>>> +u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915)
>>> +{
>>> +       return resource_size(&i915->dsm.stolen);
>>> +}
>>> +
>>> +u64 i915_gem_stolen_node_address(const struct drm_i915_private
>>> *i915,
>>> +                                const struct drm_mm_node *node)
>>> +{
>>> +       return i915->dsm.stolen.start +
>>> i915_gem_stolen_node_offset(node);
>>> +}
>>> +
>>> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
>>> *node)
>>> +{
>>> +       return drm_mm_node_allocated(node);
>>> +}
>>> +
>>> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node)
>>> +{
>>> +       return node->start;
>>> +}
>>> +
>>> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
>>> +{
>>> +       return node->size;
>>
>> Above 3 functions are core drm functions/struct and not related to
>> stolen so I don't think
>>
>> they deserve special functions in stolen code.
> Xe and i915 have differing implementations for stolen memory. We want
> to remove these details from FBC code. The thing here is that in i915
> case stolen memory node == drm mm node. In Xe case it is not and
> interfaces for these queries will have own implementation for Xe. See:
>
> https://patchwork.freedesktop.org/patch/540793/?series=118560&rev=3


Is the motivation to keep display code same for both XE and i915 without 
adding lots #if ?


Regards,

Nirmoy

>
> BR,
>
> Jouni Högander
>>
>> Regards,
>>
>> Nirmoy
>>
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>> index d5005a39d130..258381d1c054 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>> @@ -12,6 +12,8 @@ struct drm_i915_private;
>>>    struct drm_mm_node;
>>>    struct drm_i915_gem_object;
>>>    
>>> +#define i915_stolen_fb drm_mm_node
>>> +
>>>    int i915_gem_stolen_insert_node(struct drm_i915_private
>>> *dev_priv,
>>>                                  struct drm_mm_node *node, u64 size,
>>>                                  unsigned alignment);
>>> @@ -36,4 +38,15 @@ bool i915_gem_object_is_stolen(const struct
>>> drm_i915_gem_object *obj);
>>>    
>>>    #define I915_GEM_STOLEN_BIAS SZ_128K
>>>    
>>> +bool i915_gem_stolen_initialized(const struct drm_i915_private
>>> *i915);
>>> +u64 i915_gem_stolen_area_address(const struct drm_i915_private
>>> *i915);
>>> +u64 i915_gem_stolen_area_size(const struct drm_i915_private
>>> *i915);
>>> +
>>> +u64 i915_gem_stolen_node_address(const struct drm_i915_private
>>> *i915,
>>> +                                const struct drm_mm_node *node);
>>> +
>>> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
>>> *node);
>>> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node);
>>> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
>>> +
>>>    #endif /* __I915_GEM_STOLEN_H__ */
Hogander, Jouni Aug. 2, 2023, 7:52 a.m. UTC | #4
On Wed, 2023-08-02 at 09:51 +0200, Nirmoy Das wrote:
> 
> On 8/1/2023 10:33 AM, Hogander, Jouni wrote:
> > On Tue, 2023-08-01 at 10:02 +0200, Nirmoy Das wrote:
> > > Hi Jouni,
> > > 
> > > On 6/14/2023 7:17 AM, Jouni Högander wrote:
> > > > We are preparing for Xe. Xe stolen memory handling differs from
> > > > i915 so we
> > > > want to move stolen memory handling details into
> > > > i915_gem_stolen.
> > > > 
> > > > Also add a common type for fbc compressed fb and use it from
> > > > fbc
> > > > code
> > > > instead of underlying type directly. This way we can have
> > > > common
> > > > type
> > > > i915_stolen_fb for both i915 and Xe.
> > > > 
> > > > v2: Fix couple of checkpatch warnings
> > > > 
> > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > > Signed-off-by: Maarten Lankhorst
> > > > <maarten.lankhorst@linux.intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++-
> > > > -----
> > > > -----
> > > >    drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36
> > > > +++++++++++++++++
> > > >    drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
> > > >    3 files changed, 73 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > index 7f8b2d7713c7..a18e84efe911 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > @@ -94,8 +94,7 @@ struct intel_fbc {
> > > >          struct mutex lock;
> > > >          unsigned int busy_bits;
> > > >    
> > > > -       struct drm_mm_node compressed_fb;
> > > > -       struct drm_mm_node compressed_llb;
> > > > +       struct i915_stolen_fb compressed_fb, compressed_llb;
> > > >    
> > > >          enum intel_fbc_id id;
> > > >    
> > > > @@ -332,15 +331,16 @@ static void i8xx_fbc_program_cfb(struct
> > > > intel_fbc *fbc)
> > > >    {
> > > >          struct drm_i915_private *i915 = fbc->i915;
> > > >    
> > > > -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
> > > > > dsm.stolen.start,
> > > > -                                        fbc-
> > > > >compressed_fb.start,
> > > > U32_MAX));
> > > > -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
> > > > > dsm.stolen.start,
> > > > -                                        fbc-
> > > > >compressed_llb.start,
> > > > U32_MAX));
> > > > -
> > > > +       GEM_BUG_ON(range_overflows_end_t(u64,
> > > > i915_gem_stolen_area_address(i915),
> > > > +
> > > > i915_gem_stolen_node_offset(&fbc->compressed_fb),
> > > > +                                        U32_MAX));
> > > > +       GEM_BUG_ON(range_overflows_end_t(u64,
> > > > i915_gem_stolen_area_address(i915),
> > > > +
> > > > i915_gem_stolen_node_offset(&fbc->compressed_llb),
> > > > +                                        U32_MAX));
> > > >          intel_de_write(i915, FBC_CFB_BASE,
> > > > -                      i915->dsm.stolen.start + fbc-
> > > > > compressed_fb.start);
> > > > +                      i915_gem_stolen_node_address(i915, &fbc-
> > > > > compressed_fb));
> > > >          intel_de_write(i915, FBC_LL_BASE,
> > > > -                      i915->dsm.stolen.start + fbc-
> > > > > compressed_llb.start);
> > > > +                      i915_gem_stolen_node_address(i915, &fbc-
> > > > > compressed_llb));
> > > >    }
> > > >    
> > > >    static const struct intel_fbc_funcs i8xx_fbc_funcs = {
> > > > @@ -447,7 +447,8 @@ static void g4x_fbc_program_cfb(struct
> > > > intel_fbc *fbc)
> > > >    {
> > > >          struct drm_i915_private *i915 = fbc->i915;
> > > >    
> > > > -       intel_de_write(i915, DPFC_CB_BASE, fbc-
> > > > > compressed_fb.start);
> > > > +       intel_de_write(i915, DPFC_CB_BASE,
> > > > +                      i915_gem_stolen_node_offset(&fbc-
> > > > > compressed_fb));
> > > >    }
> > > >    
> > > >    static const struct intel_fbc_funcs g4x_fbc_funcs = {
> > > > @@ -498,7 +499,8 @@ static void ilk_fbc_program_cfb(struct
> > > > intel_fbc *fbc)
> > > >    {
> > > >          struct drm_i915_private *i915 = fbc->i915;
> > > >    
> > > > -       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc-
> > > > > compressed_fb.start);
> > > > +       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
> > > > +                      i915_gem_stolen_node_offset(&fbc-
> > > > > compressed_fb));
> > > >    }
> > > >    
> > > >    static const struct intel_fbc_funcs ilk_fbc_funcs = {
> > > > @@ -713,7 +715,7 @@ static u64 intel_fbc_stolen_end(struct
> > > > drm_i915_private *i915)
> > > >           * underruns, even if that range is not reserved by
> > > > the
> > > > BIOS. */
> > > >          if (IS_BROADWELL(i915) ||
> > > >              (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
> > > > -               end = resource_size(&i915->dsm.stolen) - 8 *
> > > > 1024 *
> > > > 1024;
> > > > +               end = i915_gem_stolen_area_size(i915) - 8 *
> > > > 1024 *
> > > > 1024;
> > > >          else
> > > >                  end = U64_MAX;
> > > >    
> > > > @@ -770,9 +772,9 @@ static int intel_fbc_alloc_cfb(struct
> > > > intel_fbc
> > > > *fbc,
> > > >          int ret;
> > > >    
> > > >          drm_WARN_ON(&i915->drm,
> > > > -                   drm_mm_node_allocated(&fbc-
> > > > >compressed_fb));
> > > > +                   i915_gem_stolen_node_allocated(&fbc-
> > > > > compressed_fb));
> > > >          drm_WARN_ON(&i915->drm,
> > > > -                   drm_mm_node_allocated(&fbc-
> > > > >compressed_llb));
> > > > +                   i915_gem_stolen_node_allocated(&fbc-
> > > > > compressed_llb));
> > > >    
> > > >          if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
> > > >                  ret = i915_gem_stolen_insert_node(i915, &fbc-
> > > > > compressed_llb,
> > > > @@ -792,15 +794,14 @@ static int intel_fbc_alloc_cfb(struct
> > > > intel_fbc *fbc,
> > > >    
> > > >          drm_dbg_kms(&i915->drm,
> > > >                      "reserved %llu bytes of contiguous stolen
> > > > space
> > > > for FBC, limit: %d\n",
> > > > -                   fbc->compressed_fb.size, fbc->limit);
> > > > -
> > > > +                   i915_gem_stolen_node_size(&fbc-
> > > > >compressed_fb),
> > > > fbc->limit);
> > > >          return 0;
> > > >    
> > > >    err_llb:
> > > > -       if (drm_mm_node_allocated(&fbc->compressed_llb))
> > > > +       if (i915_gem_stolen_node_allocated(&fbc-
> > > > >compressed_llb))
> > > >                  i915_gem_stolen_remove_node(i915, &fbc-
> > > > > compressed_llb);
> > > >    err:
> > > > -       if (drm_mm_initialized(&i915->mm.stolen))
> > > > +       if (i915_gem_stolen_initialized(i915))
> > > >                  drm_info_once(&i915->drm, "not enough stolen
> > > > space
> > > > for compressed buffer (need %d more bytes), disabling. Hint:
> > > > you
> > > > may be able to increase stolen memory size in the BIOS to avoid
> > > > this.\n", size);
> > > >          return -ENOSPC;
> > > >    }
> > > > @@ -825,9 +826,9 @@ static void __intel_fbc_cleanup_cfb(struct
> > > > intel_fbc *fbc)
> > > >          if (WARN_ON(intel_fbc_hw_is_active(fbc)))
> > > >                  return;
> > > >    
> > > > -       if (drm_mm_node_allocated(&fbc->compressed_llb))
> > > > +       if (i915_gem_stolen_node_allocated(&fbc-
> > > > >compressed_llb))
> > > >                  i915_gem_stolen_remove_node(i915, &fbc-
> > > > > compressed_llb);
> > > > -       if (drm_mm_node_allocated(&fbc->compressed_fb))
> > > > +       if (i915_gem_stolen_node_allocated(&fbc-
> > > > >compressed_fb))
> > > >                  i915_gem_stolen_remove_node(i915, &fbc-
> > > > > compressed_fb);
> > > >    }
> > > >    
> > > > @@ -1030,7 +1031,8 @@ static bool intel_fbc_is_cfb_ok(const
> > > > struct
> > > > intel_plane_state *plane_state)
> > > >          struct intel_fbc *fbc = plane->fbc;
> > > >    
> > > >          return intel_fbc_min_limit(plane_state) <= fbc->limit
> > > > &&
> > > > -               intel_fbc_cfb_size(plane_state) <= fbc-
> > > > > compressed_fb.size * fbc->limit;
> > > > +               intel_fbc_cfb_size(plane_state) <= fbc->limit *
> > > > +                       i915_gem_stolen_node_size(&fbc-
> > > > > compressed_fb);
> > > >    }
> > > >    
> > > >    static bool intel_fbc_is_ok(const struct intel_plane_state
> > > > *plane_state)
> > > > @@ -1707,7 +1709,7 @@ void intel_fbc_init(struct
> > > > drm_i915_private
> > > > *i915)
> > > >    {
> > > >          enum intel_fbc_id fbc_id;
> > > >    
> > > > -       if (!drm_mm_initialized(&i915->mm.stolen))
> > > > +       if (!i915_gem_stolen_initialized(i915))
> > > >                  DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
> > > >    
> > > >          if (need_fbc_vtd_wa(i915))
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > > index 3b094d36a0b0..78bac1e611dd 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> > > > @@ -974,3 +974,39 @@ bool i915_gem_object_is_stolen(const
> > > > struct
> > > > drm_i915_gem_object *obj)
> > > >    {
> > > >          return obj->ops == &i915_gem_object_stolen_ops;
> > > >    }
> > > > +
> > > > +bool i915_gem_stolen_initialized(const struct drm_i915_private
> > > > *i915)
> > > > +{
> > > > +       return drm_mm_initialized(&i915->mm.stolen);
> > > > +}
> > > > +
> > > > +u64 i915_gem_stolen_area_address(const struct drm_i915_private
> > > > *i915)
> > > > +{
> > > > +       return i915->dsm.stolen.start;
> > > > +}
> > > > +
> > > > +u64 i915_gem_stolen_area_size(const struct drm_i915_private
> > > > *i915)
> > > > +{
> > > > +       return resource_size(&i915->dsm.stolen);
> > > > +}
> > > > +
> > > > +u64 i915_gem_stolen_node_address(const struct drm_i915_private
> > > > *i915,
> > > > +                                const struct drm_mm_node
> > > > *node)
> > > > +{
> > > > +       return i915->dsm.stolen.start +
> > > > i915_gem_stolen_node_offset(node);
> > > > +}
> > > > +
> > > > +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
> > > > *node)
> > > > +{
> > > > +       return drm_mm_node_allocated(node);
> > > > +}
> > > > +
> > > > +u64 i915_gem_stolen_node_offset(const struct drm_mm_node
> > > > *node)
> > > > +{
> > > > +       return node->start;
> > > > +}
> > > > +
> > > > +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
> > > > +{
> > > > +       return node->size;
> > > 
> > > Above 3 functions are core drm functions/struct and not related
> > > to
> > > stolen so I don't think
> > > 
> > > they deserve special functions in stolen code.
> > Xe and i915 have differing implementations for stolen memory. We
> > want
> > to remove these details from FBC code. The thing here is that in
> > i915
> > case stolen memory node == drm mm node. In Xe case it is not and
> > interfaces for these queries will have own implementation for Xe.
> > See:
> > 
> > https://patchwork.freedesktop.org/patch/540793/?series=118560&rev=3
> 
> 
> Is the motivation to keep display code same for both XE and i915
> without 
> adding lots #if ?

Yes, this is our target currently.

> 
> 
> Regards,
> 
> Nirmoy
> 
> > 
> > BR,
> > 
> > Jouni Högander
> > > 
> > > Regards,
> > > 
> > > Nirmoy
> > > 
> > > > +}
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > > > b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > > > index d5005a39d130..258381d1c054 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
> > > > @@ -12,6 +12,8 @@ struct drm_i915_private;
> > > >    struct drm_mm_node;
> > > >    struct drm_i915_gem_object;
> > > >    
> > > > +#define i915_stolen_fb drm_mm_node
> > > > +
> > > >    int i915_gem_stolen_insert_node(struct drm_i915_private
> > > > *dev_priv,
> > > >                                  struct drm_mm_node *node, u64
> > > > size,
> > > >                                  unsigned alignment);
> > > > @@ -36,4 +38,15 @@ bool i915_gem_object_is_stolen(const struct
> > > > drm_i915_gem_object *obj);
> > > >    
> > > >    #define I915_GEM_STOLEN_BIAS SZ_128K
> > > >    
> > > > +bool i915_gem_stolen_initialized(const struct drm_i915_private
> > > > *i915);
> > > > +u64 i915_gem_stolen_area_address(const struct drm_i915_private
> > > > *i915);
> > > > +u64 i915_gem_stolen_area_size(const struct drm_i915_private
> > > > *i915);
> > > > +
> > > > +u64 i915_gem_stolen_node_address(const struct drm_i915_private
> > > > *i915,
> > > > +                                const struct drm_mm_node
> > > > *node);
> > > > +
> > > > +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
> > > > *node);
> > > > +u64 i915_gem_stolen_node_offset(const struct drm_mm_node
> > > > *node);
> > > > +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
> > > > +
> > > >    #endif /* __I915_GEM_STOLEN_H__ */
Nirmoy Das Aug. 3, 2023, 8:51 a.m. UTC | #5
Hi Jouni,

On 8/2/2023 9:52 AM, Hogander, Jouni wrote:
> On Wed, 2023-08-02 at 09:51 +0200, Nirmoy Das wrote:
>> On 8/1/2023 10:33 AM, Hogander, Jouni wrote:
>>> On Tue, 2023-08-01 at 10:02 +0200, Nirmoy Das wrote:
>>>> Hi Jouni,
>>>>
>>>> On 6/14/2023 7:17 AM, Jouni Högander wrote:
>>>>> We are preparing for Xe. Xe stolen memory handling differs from
>>>>> i915 so we
>>>>> want to move stolen memory handling details into
>>>>> i915_gem_stolen.
>>>>>
>>>>> Also add a common type for fbc compressed fb and use it from
>>>>> fbc
>>>>> code
>>>>> instead of underlying type directly. This way we can have
>>>>> common
>>>>> type
>>>>> i915_stolen_fb for both i915 and Xe.
>>>>>
>>>>> v2: Fix couple of checkpatch warnings
>>>>>
>>>>> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>>>>> Signed-off-by: Maarten Lankhorst
>>>>> <maarten.lankhorst@linux.intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/i915/display/intel_fbc.c   | 46 +++++++++++-
>>>>> -----
>>>>> -----
>>>>>     drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 36
>>>>> +++++++++++++++++
>>>>>     drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 13 ++++++
>>>>>     3 files changed, 73 insertions(+), 22 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
>>>>> b/drivers/gpu/drm/i915/display/intel_fbc.c
>>>>> index 7f8b2d7713c7..a18e84efe911 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>>>>> @@ -94,8 +94,7 @@ struct intel_fbc {
>>>>>           struct mutex lock;
>>>>>           unsigned int busy_bits;
>>>>>     
>>>>> -       struct drm_mm_node compressed_fb;
>>>>> -       struct drm_mm_node compressed_llb;
>>>>> +       struct i915_stolen_fb compressed_fb, compressed_llb;
>>>>>     
>>>>>           enum intel_fbc_id id;
>>>>>     
>>>>> @@ -332,15 +331,16 @@ static void i8xx_fbc_program_cfb(struct
>>>>> intel_fbc *fbc)
>>>>>     {
>>>>>           struct drm_i915_private *i915 = fbc->i915;
>>>>>     
>>>>> -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
>>>>>> dsm.stolen.start,
>>>>> -                                        fbc-
>>>>>> compressed_fb.start,
>>>>> U32_MAX));
>>>>> -       GEM_BUG_ON(range_overflows_end_t(u64, i915-
>>>>>> dsm.stolen.start,
>>>>> -                                        fbc-
>>>>>> compressed_llb.start,
>>>>> U32_MAX));
>>>>> -
>>>>> +       GEM_BUG_ON(range_overflows_end_t(u64,
>>>>> i915_gem_stolen_area_address(i915),
>>>>> +
>>>>> i915_gem_stolen_node_offset(&fbc->compressed_fb),
>>>>> +                                        U32_MAX));
>>>>> +       GEM_BUG_ON(range_overflows_end_t(u64,
>>>>> i915_gem_stolen_area_address(i915),
>>>>> +
>>>>> i915_gem_stolen_node_offset(&fbc->compressed_llb),
>>>>> +                                        U32_MAX));
>>>>>           intel_de_write(i915, FBC_CFB_BASE,
>>>>> -                      i915->dsm.stolen.start + fbc-
>>>>>> compressed_fb.start);
>>>>> +                      i915_gem_stolen_node_address(i915, &fbc-
>>>>>> compressed_fb));
>>>>>           intel_de_write(i915, FBC_LL_BASE,
>>>>> -                      i915->dsm.stolen.start + fbc-
>>>>>> compressed_llb.start);
>>>>> +                      i915_gem_stolen_node_address(i915, &fbc-
>>>>>> compressed_llb));
>>>>>     }
>>>>>     
>>>>>     static const struct intel_fbc_funcs i8xx_fbc_funcs = {
>>>>> @@ -447,7 +447,8 @@ static void g4x_fbc_program_cfb(struct
>>>>> intel_fbc *fbc)
>>>>>     {
>>>>>           struct drm_i915_private *i915 = fbc->i915;
>>>>>     
>>>>> -       intel_de_write(i915, DPFC_CB_BASE, fbc-
>>>>>> compressed_fb.start);
>>>>> +       intel_de_write(i915, DPFC_CB_BASE,
>>>>> +                      i915_gem_stolen_node_offset(&fbc-
>>>>>> compressed_fb));
>>>>>     }
>>>>>     
>>>>>     static const struct intel_fbc_funcs g4x_fbc_funcs = {
>>>>> @@ -498,7 +499,8 @@ static void ilk_fbc_program_cfb(struct
>>>>> intel_fbc *fbc)
>>>>>     {
>>>>>           struct drm_i915_private *i915 = fbc->i915;
>>>>>     
>>>>> -       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc-
>>>>>> compressed_fb.start);
>>>>> +       intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
>>>>> +                      i915_gem_stolen_node_offset(&fbc-
>>>>>> compressed_fb));
>>>>>     }
>>>>>     
>>>>>     static const struct intel_fbc_funcs ilk_fbc_funcs = {
>>>>> @@ -713,7 +715,7 @@ static u64 intel_fbc_stolen_end(struct
>>>>> drm_i915_private *i915)
>>>>>            * underruns, even if that range is not reserved by
>>>>> the
>>>>> BIOS. */
>>>>>           if (IS_BROADWELL(i915) ||
>>>>>               (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
>>>>> -               end = resource_size(&i915->dsm.stolen) - 8 *
>>>>> 1024 *
>>>>> 1024;
>>>>> +               end = i915_gem_stolen_area_size(i915) - 8 *
>>>>> 1024 *
>>>>> 1024;
>>>>>           else
>>>>>                   end = U64_MAX;
>>>>>     
>>>>> @@ -770,9 +772,9 @@ static int intel_fbc_alloc_cfb(struct
>>>>> intel_fbc
>>>>> *fbc,
>>>>>           int ret;
>>>>>     
>>>>>           drm_WARN_ON(&i915->drm,
>>>>> -                   drm_mm_node_allocated(&fbc-
>>>>>> compressed_fb));
>>>>> +                   i915_gem_stolen_node_allocated(&fbc-
>>>>>> compressed_fb));
>>>>>           drm_WARN_ON(&i915->drm,
>>>>> -                   drm_mm_node_allocated(&fbc-
>>>>>> compressed_llb));
>>>>> +                   i915_gem_stolen_node_allocated(&fbc-
>>>>>> compressed_llb));
>>>>>     
>>>>>           if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
>>>>>                   ret = i915_gem_stolen_insert_node(i915, &fbc-
>>>>>> compressed_llb,
>>>>> @@ -792,15 +794,14 @@ static int intel_fbc_alloc_cfb(struct
>>>>> intel_fbc *fbc,
>>>>>     
>>>>>           drm_dbg_kms(&i915->drm,
>>>>>                       "reserved %llu bytes of contiguous stolen
>>>>> space
>>>>> for FBC, limit: %d\n",
>>>>> -                   fbc->compressed_fb.size, fbc->limit);
>>>>> -
>>>>> +                   i915_gem_stolen_node_size(&fbc-
>>>>>> compressed_fb),
>>>>> fbc->limit);
>>>>>           return 0;
>>>>>     
>>>>>     err_llb:
>>>>> -       if (drm_mm_node_allocated(&fbc->compressed_llb))
>>>>> +       if (i915_gem_stolen_node_allocated(&fbc-
>>>>>> compressed_llb))
>>>>>                   i915_gem_stolen_remove_node(i915, &fbc-
>>>>>> compressed_llb);
>>>>>     err:
>>>>> -       if (drm_mm_initialized(&i915->mm.stolen))
>>>>> +       if (i915_gem_stolen_initialized(i915))
>>>>>                   drm_info_once(&i915->drm, "not enough stolen
>>>>> space
>>>>> for compressed buffer (need %d more bytes), disabling. Hint:
>>>>> you
>>>>> may be able to increase stolen memory size in the BIOS to avoid
>>>>> this.\n", size);
>>>>>           return -ENOSPC;
>>>>>     }
>>>>> @@ -825,9 +826,9 @@ static void __intel_fbc_cleanup_cfb(struct
>>>>> intel_fbc *fbc)
>>>>>           if (WARN_ON(intel_fbc_hw_is_active(fbc)))
>>>>>                   return;
>>>>>     
>>>>> -       if (drm_mm_node_allocated(&fbc->compressed_llb))
>>>>> +       if (i915_gem_stolen_node_allocated(&fbc-
>>>>>> compressed_llb))
>>>>>                   i915_gem_stolen_remove_node(i915, &fbc-
>>>>>> compressed_llb);
>>>>> -       if (drm_mm_node_allocated(&fbc->compressed_fb))
>>>>> +       if (i915_gem_stolen_node_allocated(&fbc-
>>>>>> compressed_fb))
>>>>>                   i915_gem_stolen_remove_node(i915, &fbc-
>>>>>> compressed_fb);
>>>>>     }
>>>>>     
>>>>> @@ -1030,7 +1031,8 @@ static bool intel_fbc_is_cfb_ok(const
>>>>> struct
>>>>> intel_plane_state *plane_state)
>>>>>           struct intel_fbc *fbc = plane->fbc;
>>>>>     
>>>>>           return intel_fbc_min_limit(plane_state) <= fbc->limit
>>>>> &&
>>>>> -               intel_fbc_cfb_size(plane_state) <= fbc-
>>>>>> compressed_fb.size * fbc->limit;
>>>>> +               intel_fbc_cfb_size(plane_state) <= fbc->limit *
>>>>> +                       i915_gem_stolen_node_size(&fbc-
>>>>>> compressed_fb);
>>>>>     }
>>>>>     
>>>>>     static bool intel_fbc_is_ok(const struct intel_plane_state
>>>>> *plane_state)
>>>>> @@ -1707,7 +1709,7 @@ void intel_fbc_init(struct
>>>>> drm_i915_private
>>>>> *i915)
>>>>>     {
>>>>>           enum intel_fbc_id fbc_id;
>>>>>     
>>>>> -       if (!drm_mm_initialized(&i915->mm.stolen))
>>>>> +       if (!i915_gem_stolen_initialized(i915))
>>>>>                   DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
>>>>>     
>>>>>           if (need_fbc_vtd_wa(i915))
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>>>> index 3b094d36a0b0..78bac1e611dd 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>>>> @@ -974,3 +974,39 @@ bool i915_gem_object_is_stolen(const
>>>>> struct
>>>>> drm_i915_gem_object *obj)
>>>>>     {
>>>>>           return obj->ops == &i915_gem_object_stolen_ops;
>>>>>     }
>>>>> +
>>>>> +bool i915_gem_stolen_initialized(const struct drm_i915_private
>>>>> *i915)
>>>>> +{
>>>>> +       return drm_mm_initialized(&i915->mm.stolen);
>>>>> +}
>>>>> +
>>>>> +u64 i915_gem_stolen_area_address(const struct drm_i915_private
>>>>> *i915)
>>>>> +{
>>>>> +       return i915->dsm.stolen.start;
>>>>> +}
>>>>> +
>>>>> +u64 i915_gem_stolen_area_size(const struct drm_i915_private
>>>>> *i915)
>>>>> +{
>>>>> +       return resource_size(&i915->dsm.stolen);
>>>>> +}
>>>>> +
>>>>> +u64 i915_gem_stolen_node_address(const struct drm_i915_private
>>>>> *i915,
>>>>> +                                const struct drm_mm_node
>>>>> *node)
>>>>> +{
>>>>> +       return i915->dsm.stolen.start +
>>>>> i915_gem_stolen_node_offset(node);
>>>>> +}
>>>>> +
>>>>> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
>>>>> *node)
>>>>> +{
>>>>> +       return drm_mm_node_allocated(node);
>>>>> +}
>>>>> +
>>>>> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node
>>>>> *node)
>>>>> +{
>>>>> +       return node->start;
>>>>> +}
>>>>> +
>>>>> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
>>>>> +{
>>>>> +       return node->size;
>>>> Above 3 functions are core drm functions/struct and not related
>>>> to
>>>> stolen so I don't think
>>>>
>>>> they deserve special functions in stolen code.
>>> Xe and i915 have differing implementations for stolen memory. We
>>> want
>>> to remove these details from FBC code. The thing here is that in
>>> i915
>>> case stolen memory node == drm mm node. In Xe case it is not and
>>> interfaces for these queries will have own implementation for Xe.
>>> See:
>>>
>>> https://patchwork.freedesktop.org/patch/540793/?series=118560&rev=3
>>
>> Is the motivation to keep display code same for both XE and i915
>> without
>> adding lots #if ?
> Yes, this is our target currently.

Got it, in that case

Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>

Regards,

Nirmoy

>
>>
>> Regards,
>>
>> Nirmoy
>>
>>> BR,
>>>
>>> Jouni Högander
>>>> Regards,
>>>>
>>>> Nirmoy
>>>>
>>>>> +}
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>>>> index d5005a39d130..258381d1c054 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
>>>>> @@ -12,6 +12,8 @@ struct drm_i915_private;
>>>>>     struct drm_mm_node;
>>>>>     struct drm_i915_gem_object;
>>>>>     
>>>>> +#define i915_stolen_fb drm_mm_node
>>>>> +
>>>>>     int i915_gem_stolen_insert_node(struct drm_i915_private
>>>>> *dev_priv,
>>>>>                                   struct drm_mm_node *node, u64
>>>>> size,
>>>>>                                   unsigned alignment);
>>>>> @@ -36,4 +38,15 @@ bool i915_gem_object_is_stolen(const struct
>>>>> drm_i915_gem_object *obj);
>>>>>     
>>>>>     #define I915_GEM_STOLEN_BIAS SZ_128K
>>>>>     
>>>>> +bool i915_gem_stolen_initialized(const struct drm_i915_private
>>>>> *i915);
>>>>> +u64 i915_gem_stolen_area_address(const struct drm_i915_private
>>>>> *i915);
>>>>> +u64 i915_gem_stolen_area_size(const struct drm_i915_private
>>>>> *i915);
>>>>> +
>>>>> +u64 i915_gem_stolen_node_address(const struct drm_i915_private
>>>>> *i915,
>>>>> +                                const struct drm_mm_node
>>>>> *node);
>>>>> +
>>>>> +bool i915_gem_stolen_node_allocated(const struct drm_mm_node
>>>>> *node);
>>>>> +u64 i915_gem_stolen_node_offset(const struct drm_mm_node
>>>>> *node);
>>>>> +u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
>>>>> +
>>>>>     #endif /* __I915_GEM_STOLEN_H__ */
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 7f8b2d7713c7..a18e84efe911 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -94,8 +94,7 @@  struct intel_fbc {
 	struct mutex lock;
 	unsigned int busy_bits;
 
-	struct drm_mm_node compressed_fb;
-	struct drm_mm_node compressed_llb;
+	struct i915_stolen_fb compressed_fb, compressed_llb;
 
 	enum intel_fbc_id id;
 
@@ -332,15 +331,16 @@  static void i8xx_fbc_program_cfb(struct intel_fbc *fbc)
 {
 	struct drm_i915_private *i915 = fbc->i915;
 
-	GEM_BUG_ON(range_overflows_end_t(u64, i915->dsm.stolen.start,
-					 fbc->compressed_fb.start, U32_MAX));
-	GEM_BUG_ON(range_overflows_end_t(u64, i915->dsm.stolen.start,
-					 fbc->compressed_llb.start, U32_MAX));
-
+	GEM_BUG_ON(range_overflows_end_t(u64, i915_gem_stolen_area_address(i915),
+					 i915_gem_stolen_node_offset(&fbc->compressed_fb),
+					 U32_MAX));
+	GEM_BUG_ON(range_overflows_end_t(u64, i915_gem_stolen_area_address(i915),
+					 i915_gem_stolen_node_offset(&fbc->compressed_llb),
+					 U32_MAX));
 	intel_de_write(i915, FBC_CFB_BASE,
-		       i915->dsm.stolen.start + fbc->compressed_fb.start);
+		       i915_gem_stolen_node_address(i915, &fbc->compressed_fb));
 	intel_de_write(i915, FBC_LL_BASE,
-		       i915->dsm.stolen.start + fbc->compressed_llb.start);
+		       i915_gem_stolen_node_address(i915, &fbc->compressed_llb));
 }
 
 static const struct intel_fbc_funcs i8xx_fbc_funcs = {
@@ -447,7 +447,8 @@  static void g4x_fbc_program_cfb(struct intel_fbc *fbc)
 {
 	struct drm_i915_private *i915 = fbc->i915;
 
-	intel_de_write(i915, DPFC_CB_BASE, fbc->compressed_fb.start);
+	intel_de_write(i915, DPFC_CB_BASE,
+		       i915_gem_stolen_node_offset(&fbc->compressed_fb));
 }
 
 static const struct intel_fbc_funcs g4x_fbc_funcs = {
@@ -498,7 +499,8 @@  static void ilk_fbc_program_cfb(struct intel_fbc *fbc)
 {
 	struct drm_i915_private *i915 = fbc->i915;
 
-	intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id), fbc->compressed_fb.start);
+	intel_de_write(i915, ILK_DPFC_CB_BASE(fbc->id),
+		       i915_gem_stolen_node_offset(&fbc->compressed_fb));
 }
 
 static const struct intel_fbc_funcs ilk_fbc_funcs = {
@@ -713,7 +715,7 @@  static u64 intel_fbc_stolen_end(struct drm_i915_private *i915)
 	 * underruns, even if that range is not reserved by the BIOS. */
 	if (IS_BROADWELL(i915) ||
 	    (DISPLAY_VER(i915) == 9 && !IS_BROXTON(i915)))
-		end = resource_size(&i915->dsm.stolen) - 8 * 1024 * 1024;
+		end = i915_gem_stolen_area_size(i915) - 8 * 1024 * 1024;
 	else
 		end = U64_MAX;
 
@@ -770,9 +772,9 @@  static int intel_fbc_alloc_cfb(struct intel_fbc *fbc,
 	int ret;
 
 	drm_WARN_ON(&i915->drm,
-		    drm_mm_node_allocated(&fbc->compressed_fb));
+		    i915_gem_stolen_node_allocated(&fbc->compressed_fb));
 	drm_WARN_ON(&i915->drm,
-		    drm_mm_node_allocated(&fbc->compressed_llb));
+		    i915_gem_stolen_node_allocated(&fbc->compressed_llb));
 
 	if (DISPLAY_VER(i915) < 5 && !IS_G4X(i915)) {
 		ret = i915_gem_stolen_insert_node(i915, &fbc->compressed_llb,
@@ -792,15 +794,14 @@  static int intel_fbc_alloc_cfb(struct intel_fbc *fbc,
 
 	drm_dbg_kms(&i915->drm,
 		    "reserved %llu bytes of contiguous stolen space for FBC, limit: %d\n",
-		    fbc->compressed_fb.size, fbc->limit);
-
+		    i915_gem_stolen_node_size(&fbc->compressed_fb), fbc->limit);
 	return 0;
 
 err_llb:
-	if (drm_mm_node_allocated(&fbc->compressed_llb))
+	if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
 		i915_gem_stolen_remove_node(i915, &fbc->compressed_llb);
 err:
-	if (drm_mm_initialized(&i915->mm.stolen))
+	if (i915_gem_stolen_initialized(i915))
 		drm_info_once(&i915->drm, "not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
 	return -ENOSPC;
 }
@@ -825,9 +826,9 @@  static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
 	if (WARN_ON(intel_fbc_hw_is_active(fbc)))
 		return;
 
-	if (drm_mm_node_allocated(&fbc->compressed_llb))
+	if (i915_gem_stolen_node_allocated(&fbc->compressed_llb))
 		i915_gem_stolen_remove_node(i915, &fbc->compressed_llb);
-	if (drm_mm_node_allocated(&fbc->compressed_fb))
+	if (i915_gem_stolen_node_allocated(&fbc->compressed_fb))
 		i915_gem_stolen_remove_node(i915, &fbc->compressed_fb);
 }
 
@@ -1030,7 +1031,8 @@  static bool intel_fbc_is_cfb_ok(const struct intel_plane_state *plane_state)
 	struct intel_fbc *fbc = plane->fbc;
 
 	return intel_fbc_min_limit(plane_state) <= fbc->limit &&
-		intel_fbc_cfb_size(plane_state) <= fbc->compressed_fb.size * fbc->limit;
+		intel_fbc_cfb_size(plane_state) <= fbc->limit *
+			i915_gem_stolen_node_size(&fbc->compressed_fb);
 }
 
 static bool intel_fbc_is_ok(const struct intel_plane_state *plane_state)
@@ -1707,7 +1709,7 @@  void intel_fbc_init(struct drm_i915_private *i915)
 {
 	enum intel_fbc_id fbc_id;
 
-	if (!drm_mm_initialized(&i915->mm.stolen))
+	if (!i915_gem_stolen_initialized(i915))
 		DISPLAY_RUNTIME_INFO(i915)->fbc_mask = 0;
 
 	if (need_fbc_vtd_wa(i915))
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 3b094d36a0b0..78bac1e611dd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -974,3 +974,39 @@  bool i915_gem_object_is_stolen(const struct drm_i915_gem_object *obj)
 {
 	return obj->ops == &i915_gem_object_stolen_ops;
 }
+
+bool i915_gem_stolen_initialized(const struct drm_i915_private *i915)
+{
+	return drm_mm_initialized(&i915->mm.stolen);
+}
+
+u64 i915_gem_stolen_area_address(const struct drm_i915_private *i915)
+{
+	return i915->dsm.stolen.start;
+}
+
+u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915)
+{
+	return resource_size(&i915->dsm.stolen);
+}
+
+u64 i915_gem_stolen_node_address(const struct drm_i915_private *i915,
+				 const struct drm_mm_node *node)
+{
+	return i915->dsm.stolen.start + i915_gem_stolen_node_offset(node);
+}
+
+bool i915_gem_stolen_node_allocated(const struct drm_mm_node *node)
+{
+	return drm_mm_node_allocated(node);
+}
+
+u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node)
+{
+	return node->start;
+}
+
+u64 i915_gem_stolen_node_size(const struct drm_mm_node *node)
+{
+	return node->size;
+}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
index d5005a39d130..258381d1c054 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.h
@@ -12,6 +12,8 @@  struct drm_i915_private;
 struct drm_mm_node;
 struct drm_i915_gem_object;
 
+#define i915_stolen_fb drm_mm_node
+
 int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
 				struct drm_mm_node *node, u64 size,
 				unsigned alignment);
@@ -36,4 +38,15 @@  bool i915_gem_object_is_stolen(const struct drm_i915_gem_object *obj);
 
 #define I915_GEM_STOLEN_BIAS SZ_128K
 
+bool i915_gem_stolen_initialized(const struct drm_i915_private *i915);
+u64 i915_gem_stolen_area_address(const struct drm_i915_private *i915);
+u64 i915_gem_stolen_area_size(const struct drm_i915_private *i915);
+
+u64 i915_gem_stolen_node_address(const struct drm_i915_private *i915,
+				 const struct drm_mm_node *node);
+
+bool i915_gem_stolen_node_allocated(const struct drm_mm_node *node);
+u64 i915_gem_stolen_node_offset(const struct drm_mm_node *node);
+u64 i915_gem_stolen_node_size(const struct drm_mm_node *node);
+
 #endif /* __I915_GEM_STOLEN_H__ */