Re: [PATCH 04/25] drm/i915/fbc: introduce struct intel_fbc_reg_params

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

Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> The early return inside __intel_fbc_update does not completely check
> all the parameters that affect the FBC register values. For example,
> we currently lack looking at crtc->adjusted_y (for the fence Y offset)
> and all the parameters that affect the CFB size (for i8xx).
>
> Instead of just adding the missing parameters to the check and hoping
> that any changes to the fbc_activate functions also come with a
> matching change to the __intel_fbc_update check, introduce a new
> structure where we store these parameters and use the structure at the
> fbc_activate function. Of course, it's still possible to access
> everything from dev_priv in those functions, but IMHO the new code
> will be harder to break.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  22 ++++++-
>  drivers/gpu/drm/i915/intel_fbc.c | 131 ++++++++++++++++++++++-----------------
>  2 files changed, 93 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33217a4..aa9c35e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -909,11 +909,9 @@ struct i915_fbc {
>  	 * it's the outer lock when overlapping with stolen_lock. */
>  	struct mutex lock;
>  	unsigned threshold;
> -	unsigned int fb_id;
>  	unsigned int possible_framebuffer_bits;
>  	unsigned int busy_bits;
>  	struct intel_crtc *crtc;
> -	int y;
>  
>  	struct drm_mm_node compressed_fb;
>  	struct drm_mm_node *compressed_llb;
> @@ -923,6 +921,24 @@ struct i915_fbc {
>  	bool enabled;
>  	bool active;
>  
> +	struct intel_fbc_reg_params {
> +		struct {
> +			enum pipe pipe;
> +			enum plane plane;
> +			unsigned int fence_y_offset;
> +		} crtc;
> +
> +		struct {
> +			u64 ggtt_offset;
> +			uint32_t id;
On a casual glance it looks like this fb.id is write-only now, but the whole struct is being compared.

It might be worth making a note about that.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx