Re: [PATCH 03/22] drm/i915/fb: convert parts of intel_fb.c to struct drm_gem_object

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Den 2024-09-17 kl. 18:13, skrev Jani Nikula:
> Prefer the driver agnostic struct drm_gem_object over i915 specific
> struct drm_i915_gem_object. Add new intel_bo_* functions as needed.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bo.c | 10 +++++++++
>  drivers/gpu/drm/i915/display/intel_bo.h |  2 ++
>  drivers/gpu/drm/i915/display/intel_fb.c | 29 +++++++++++++------------
>  drivers/gpu/drm/xe/display/intel_bo.c   | 10 +++++++++
>  4 files changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bo.c b/drivers/gpu/drm/i915/display/intel_bo.c
> index fede0a545951..5708d9bab1ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_bo.c
> +++ b/drivers/gpu/drm/i915/display/intel_bo.c
> @@ -8,3 +8,13 @@ bool intel_bo_is_tiled(struct drm_gem_object *obj)
>  {
>  	return i915_gem_object_is_tiled(to_intel_bo(obj));
>  }
> +
> +bool intel_bo_is_userptr(struct drm_gem_object *obj)
> +{
> +	return i915_gem_object_is_userptr(to_intel_bo(obj));
> +}
> +
> +void intel_bo_flush_if_display(struct drm_gem_object *obj)
> +{
> +	i915_gem_object_flush_if_display(to_intel_bo(obj));
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_bo.h b/drivers/gpu/drm/i915/display/intel_bo.h
> index f5dfcc16d33e..0fa29211a9d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_bo.h
> +++ b/drivers/gpu/drm/i915/display/intel_bo.h
> @@ -9,5 +9,7 @@
>  struct drm_gem_object;
>  
>  bool intel_bo_is_tiled(struct drm_gem_object *obj);
> +bool intel_bo_is_userptr(struct drm_gem_object *obj);
> +void intel_bo_flush_if_display(struct drm_gem_object *obj);
>  
>  #endif /* __INTEL_BO__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index 834a49395638..eb8dc3dd21ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -13,6 +13,7 @@
>  #include "gem/i915_gem_object_types.h"
>  #include "i915_drv.h"
>  #include "intel_atomic_plane.h"
> +#include "intel_bo.h"
>  #include "intel_display.h"
>  #include "intel_display_types.h"
>  #include "intel_dpt.h"
> @@ -1225,7 +1226,7 @@ static bool intel_plane_needs_remap(const struct intel_plane_state *plane_state)
>  static int convert_plane_offset_to_xy(const struct intel_framebuffer *fb, int color_plane,
>  				      int plane_width, int *x, int *y)
>  {
> -	struct drm_i915_gem_object *obj = intel_fb_obj(&fb->base);
> +	struct drm_gem_object *obj = intel_fb_bo(&fb->base);
>  	int ret;
>  
>  	ret = intel_fb_offset_to_xy(x, y, &fb->base, color_plane);
> @@ -1249,7 +1250,7 @@ static int convert_plane_offset_to_xy(const struct intel_framebuffer *fb, int co
>  	 * fb layout agrees with the fence layout. We already check that the
>  	 * fb stride matches the fence stride elsewhere.
>  	 */
> -	if (color_plane == 0 && i915_gem_object_is_tiled(obj) &&
> +	if (color_plane == 0 && intel_bo_is_tiled(obj) &&
>  	    (*x + plane_width) * fb->base.format->cpp[color_plane] > fb->base.pitches[color_plane]) {
>  		drm_dbg_kms(fb->base.dev,
>  			    "bad fb plane %d offset: 0x%x\n",
> @@ -1569,7 +1570,7 @@ static unsigned int intel_fb_min_alignment(const struct drm_framebuffer *fb)
>  
>  int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *fb)
>  {
> -	struct drm_i915_gem_object *obj = intel_fb_obj(&fb->base);
> +	struct drm_gem_object *obj = intel_fb_bo(&fb->base);
>  	u32 gtt_offset_rotated = 0;
>  	u32 gtt_offset_remapped = 0;
>  	unsigned int max_size = 0;
> @@ -1642,10 +1643,10 @@ int intel_fill_fb_info(struct drm_i915_private *i915, struct intel_framebuffer *
>  		max_size = max(max_size, offset + size);
>  	}
>  
> -	if (mul_u32_u32(max_size, tile_size) > intel_bo_to_drm_bo(obj)->size) {
> +	if (mul_u32_u32(max_size, tile_size) > obj->size) {
>  		drm_dbg_kms(&i915->drm,
>  			    "fb too big for bo (need %llu bytes, have %zu bytes)\n",
> -			    mul_u32_u32(max_size, tile_size), intel_bo_to_drm_bo(obj)->size);
> +			    mul_u32_u32(max_size, tile_size), obj->size);
>  		return -EINVAL;
>  	}
>  
> @@ -1878,16 +1879,16 @@ static int intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
>  						struct drm_file *file,
>  						unsigned int *handle)
>  {
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct drm_i915_private *i915 = to_i915(intel_bo_to_drm_bo(obj)->dev);
> +	struct drm_gem_object *obj = intel_fb_bo(fb);
> +	struct intel_display *display = to_intel_display(obj->dev);
>  
> -	if (i915_gem_object_is_userptr(obj)) {
> -		drm_dbg(&i915->drm,
> +	if (intel_bo_is_userptr(obj)) {
> +		drm_dbg(display->drm,
>  			"attempting to use a userptr for a framebuffer, denied\n");
>  		return -EINVAL;
>  	}
>  
> -	return drm_gem_handle_create(file, intel_bo_to_drm_bo(obj), handle);
> +	return drm_gem_handle_create(file, obj, handle);
>  }
>  
>  struct frontbuffer_fence_cb {
> @@ -1911,7 +1912,7 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
>  					struct drm_clip_rect *clips,
>  					unsigned int num_clips)
>  {
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> +	struct drm_gem_object *obj = intel_fb_bo(fb);
>  	struct intel_frontbuffer *front = to_intel_frontbuffer(fb);
>  	struct dma_fence *fence;
>  	struct frontbuffer_fence_cb *cb;
> @@ -1920,10 +1921,10 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
>  	if (!atomic_read(&front->bits))
>  		return 0;
>  
> -	if (dma_resv_test_signaled(intel_bo_to_drm_bo(obj)->resv, dma_resv_usage_rw(false)))
> +	if (dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(false)))
>  		goto flush;
>  
> -	ret = dma_resv_get_singleton(intel_bo_to_drm_bo(obj)->resv, dma_resv_usage_rw(false),
> +	ret = dma_resv_get_singleton(obj->resv, dma_resv_usage_rw(false),
>  				     &fence);
>  	if (ret || !fence)
>  		goto flush;
> @@ -1950,7 +1951,7 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
>  	return ret;
>  
>  flush:
> -	i915_gem_object_flush_if_display(obj);
> +	intel_bo_flush_if_display(obj);
>  	intel_frontbuffer_flush(front, ORIGIN_DIRTYFB);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/xe/display/intel_bo.c b/drivers/gpu/drm/xe/display/intel_bo.c
> index d564fb7d85e5..f57b684050da 100644
> --- a/drivers/gpu/drm/xe/display/intel_bo.c
> +++ b/drivers/gpu/drm/xe/display/intel_bo.c
> @@ -10,3 +10,13 @@ bool intel_bo_is_tiled(struct drm_gem_object *obj)
>  	/* legacy tiling is unused */
>  	return false;
>  }
> +
> +bool intel_bo_is_userptr(struct drm_gem_object *obj)
> +{
> +	/* legacy tiling is unused */
This comment should be 'xe does not have userptr bos'. :)
> +	return false;
> +}
> +
> +void intel_bo_flush_if_display(struct drm_gem_object *obj)
> +{
> +}