[Intel-gfx] [PATCH 2/4] drm/i915: Move GEM BO inside drm_framebuffer

[Ville Syrjälä]

On Fri, Mar 23, 2018 at 01:45:50PM +0000, Daniel Stone wrote:
> Since drm_framebuffer can now store GEM objects directly, place them
> there rather than in our own subclass.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +--
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 49b0772e9abc..e8100a4fc877 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13916,7 +13916,8 @@ static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
>  	drm_framebuffer_cleanup(fb);
>  
>  	i915_gem_object_lock(obj);
> -	WARN_ON(!obj->framebuffer_references--);
> +	WARN_ON(obj->framebuffer_references < fb->format->num_planes);
> +	obj->framebuffer_references -= fb->format->num_planes;

Hmm. I'm thinking we can stick to the single reference per fb.
IIRC this counter is there just to prevent changes of the obj
tiling mode as long as any fb exists that uses the object. So
doesn't actually matter how many planes the fb has.

Naturally the story would be slightly difference if we supported
fbs using multiple different BOs, as each BO would need to get its
framebuffer_references adjusted.

>  	i915_gem_object_unlock(obj);
>  
>  	i915_gem_object_put(obj);
> @@ -14176,9 +14177,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  				      i, fb->pitches[i], stride_alignment);
>  			goto err;
>  		}
> -	}
>  
> -	intel_fb->obj = obj;
> +		fb->obj[i] = &obj->base;
> +	}
>  
>  	ret = intel_fill_fb_info(dev_priv, fb);
>  	if (ret)
> @@ -14190,6 +14191,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  		goto err;
>  	}
>  
> +	/* We already hold one reference to the fb, but in case it's
> +	 * multi-planar and we've placed multiple pointers in
> +	 * drm_framebuffer, hold extra refs.
> +	 */
> +	i915_gem_object_lock(obj);
> +	obj->framebuffer_references += fb->format->num_planes - 1;
> +	i915_gem_object_unlock(obj);
> +
>  	return 0;
>  
>  err:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 570f89b31544..d93ed9e59867 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -186,7 +186,6 @@ enum intel_output_type {
>  
>  struct intel_framebuffer {
>  	struct drm_framebuffer base;
> -	struct drm_i915_gem_object *obj;
>  	struct intel_rotation_info rot_info;
>  
>  	/* for each plane in the normal GTT view */
> @@ -985,7 +984,7 @@ struct cxsr_latency {
>  #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
>  #define to_intel_plane(x) container_of(x, struct intel_plane, base)
>  #define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base)
> -#define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL)
> +#define intel_fb_obj(x) (((x) && (x)->obj[0]) ? to_intel_bo((x)->obj[0]) : NULL)

Ah, I've had that "(x)" part coded up so many times, but never sent it
out :)

>  
>  struct intel_hdmi {
>  	i915_reg_t hdmi_reg;
> -- 
> 2.16.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC