diff mbox series

[v10,5/8] drm/i915: Add dedicated plane hook for async flip case

Message ID 20200918070045.9703-1-karthik.b.s@intel.com
State New, archived
Headers show
Series None | expand

Commit Message

Karthik B S Sept. 18, 2020, 7 a.m. UTC
This hook is added to avoid writing other plane registers in case of
async flips, so that we do not write the double buffered registers
during async surface address update.

v7: -Plane ctl needs bits from skl_plane_ctl_crtc as well. (Ville)
    -Add a vfunc for skl_program_async_surface_address
     and call it from intel_update_plane. (Ville)

v8: -Rebased.

v9: -Use if-else instead of return in intel_update_plane(). (Ville)
    -Rename 'program_async_surface_address' to 'async_flip'. (Ville)

v10: -Check if async_flip hook is present before calling it.
      Otherwise it will OOPS during legacy cursor updates. (Ville)

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c |  6 ++++-
 .../drm/i915/display/intel_display_types.h    |  3 +++
 drivers/gpu/drm/i915/display/intel_sprite.c   | 24 +++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Sept. 18, 2020, 11:53 a.m. UTC | #1
On Fri, Sep 18, 2020 at 12:30:45PM +0530, Karthik B S wrote:
> This hook is added to avoid writing other plane registers in case of
> async flips, so that we do not write the double buffered registers
> during async surface address update.
> 
> v7: -Plane ctl needs bits from skl_plane_ctl_crtc as well. (Ville)
>     -Add a vfunc for skl_program_async_surface_address
>      and call it from intel_update_plane. (Ville)
> 
> v8: -Rebased.
> 
> v9: -Use if-else instead of return in intel_update_plane(). (Ville)
>     -Rename 'program_async_surface_address' to 'async_flip'. (Ville)
> 
> v10: -Check if async_flip hook is present before calling it.
>       Otherwise it will OOPS during legacy cursor updates. (Ville)
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.c |  6 ++++-
>  .../drm/i915/display/intel_display_types.h    |  3 +++
>  drivers/gpu/drm/i915/display/intel_sprite.c   | 24 +++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 79032701873a..6bd8e6cdd477 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -408,7 +408,11 @@ void intel_update_plane(struct intel_plane *plane,
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  
>  	trace_intel_update_plane(&plane->base, crtc);
> -	plane->update_plane(plane, crtc_state, plane_state);
> +
> +	if (crtc_state->uapi.async_flip && plane->async_flip)
> +		plane->async_flip(plane, crtc_state, plane_state);
> +	else
> +		plane->update_plane(plane, crtc_state, plane_state);
>  }
>  
>  void intel_disable_plane(struct intel_plane *plane,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 3d4bf9b6a0a2..e3339e41ddf7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1183,6 +1183,9 @@ struct intel_plane {
>  			   struct intel_plane_state *plane_state);
>  	int (*min_cdclk)(const struct intel_crtc_state *crtc_state,
>  			 const struct intel_plane_state *plane_state);
> +	void (*async_flip)(struct intel_plane *plane,
> +			   const struct intel_crtc_state *crtc_state,
> +			   const struct intel_plane_state *plane_state);
>  };
>  
>  struct intel_watermark_params {
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 76a3d9bfe0de..3634e98b04c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -609,6 +609,29 @@ icl_program_input_csc(struct intel_plane *plane,
>  			  PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
>  }
>  
> +static void
> +skl_program_async_surface_address(struct intel_plane *plane,
> +				  const struct intel_crtc_state *crtc_state,
> +				  const struct intel_plane_state *plane_state)

Pls rename this to skl_plane_async_flip() as well.

With that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +{
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	unsigned long irqflags;
> +	enum plane_id plane_id = plane->id;
> +	enum pipe pipe = plane->pipe;
> +	u32 surf_addr = plane_state->color_plane[0].offset;
> +	u32 plane_ctl = plane_state->ctl;
> +
> +	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
> +
> +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> +
> +	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
> +	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
> +			  intel_plane_ggtt_offset(plane_state) + surf_addr);
> +
> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +}
> +
>  static void
>  skl_program_plane(struct intel_plane *plane,
>  		  const struct intel_crtc_state *crtc_state,
> @@ -3095,6 +3118,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  	plane->get_hw_state = skl_plane_get_hw_state;
>  	plane->check_plane = skl_plane_check;
>  	plane->min_cdclk = skl_plane_min_cdclk;
> +	plane->async_flip = skl_program_async_surface_address;
>  
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		formats = icl_get_plane_formats(dev_priv, pipe,
> -- 
> 2.22.0
Karthik B S Sept. 21, 2020, 10:52 a.m. UTC | #2
On 9/18/2020 5:23 PM, Ville Syrjälä wrote:
> On Fri, Sep 18, 2020 at 12:30:45PM +0530, Karthik B S wrote:
>> This hook is added to avoid writing other plane registers in case of
>> async flips, so that we do not write the double buffered registers
>> during async surface address update.
>>
>> v7: -Plane ctl needs bits from skl_plane_ctl_crtc as well. (Ville)
>>      -Add a vfunc for skl_program_async_surface_address
>>       and call it from intel_update_plane. (Ville)
>>
>> v8: -Rebased.
>>
>> v9: -Use if-else instead of return in intel_update_plane(). (Ville)
>>      -Rename 'program_async_surface_address' to 'async_flip'. (Ville)
>>
>> v10: -Check if async_flip hook is present before calling it.
>>        Otherwise it will OOPS during legacy cursor updates. (Ville)
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> ---
>>   .../gpu/drm/i915/display/intel_atomic_plane.c |  6 ++++-
>>   .../drm/i915/display/intel_display_types.h    |  3 +++
>>   drivers/gpu/drm/i915/display/intel_sprite.c   | 24 +++++++++++++++++++
>>   3 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> index 79032701873a..6bd8e6cdd477 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> @@ -408,7 +408,11 @@ void intel_update_plane(struct intel_plane *plane,
>>   	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>   
>>   	trace_intel_update_plane(&plane->base, crtc);
>> -	plane->update_plane(plane, crtc_state, plane_state);
>> +
>> +	if (crtc_state->uapi.async_flip && plane->async_flip)
>> +		plane->async_flip(plane, crtc_state, plane_state);
>> +	else
>> +		plane->update_plane(plane, crtc_state, plane_state);
>>   }
>>   
>>   void intel_disable_plane(struct intel_plane *plane,
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 3d4bf9b6a0a2..e3339e41ddf7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1183,6 +1183,9 @@ struct intel_plane {
>>   			   struct intel_plane_state *plane_state);
>>   	int (*min_cdclk)(const struct intel_crtc_state *crtc_state,
>>   			 const struct intel_plane_state *plane_state);
>> +	void (*async_flip)(struct intel_plane *plane,
>> +			   const struct intel_crtc_state *crtc_state,
>> +			   const struct intel_plane_state *plane_state);
>>   };
>>   
>>   struct intel_watermark_params {
>> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
>> index 76a3d9bfe0de..3634e98b04c1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>> @@ -609,6 +609,29 @@ icl_program_input_csc(struct intel_plane *plane,
>>   			  PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
>>   }
>>   
>> +static void
>> +skl_program_async_surface_address(struct intel_plane *plane,
>> +				  const struct intel_crtc_state *crtc_state,
>> +				  const struct intel_plane_state *plane_state)
> 
> Pls rename this to skl_plane_async_flip() as well.

Thanks for the review.
Sure, I'll update this.
> 
> With that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 

Thanks,
Karthik.B.S
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>> +	unsigned long irqflags;
>> +	enum plane_id plane_id = plane->id;
>> +	enum pipe pipe = plane->pipe;
>> +	u32 surf_addr = plane_state->color_plane[0].offset;
>> +	u32 plane_ctl = plane_state->ctl;
>> +
>> +	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
>> +
>> +	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>> +
>> +	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
>> +	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
>> +			  intel_plane_ggtt_offset(plane_state) + surf_addr);
>> +
>> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>> +}
>> +
>>   static void
>>   skl_program_plane(struct intel_plane *plane,
>>   		  const struct intel_crtc_state *crtc_state,
>> @@ -3095,6 +3118,7 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>>   	plane->get_hw_state = skl_plane_get_hw_state;
>>   	plane->check_plane = skl_plane_check;
>>   	plane->min_cdclk = skl_plane_min_cdclk;
>> +	plane->async_flip = skl_program_async_surface_address;
>>   
>>   	if (INTEL_GEN(dev_priv) >= 11)
>>   		formats = icl_get_plane_formats(dev_priv, pipe,
>> -- 
>> 2.22.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 79032701873a..6bd8e6cdd477 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -408,7 +408,11 @@  void intel_update_plane(struct intel_plane *plane,
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
 	trace_intel_update_plane(&plane->base, crtc);
-	plane->update_plane(plane, crtc_state, plane_state);
+
+	if (crtc_state->uapi.async_flip && plane->async_flip)
+		plane->async_flip(plane, crtc_state, plane_state);
+	else
+		plane->update_plane(plane, crtc_state, plane_state);
 }
 
 void intel_disable_plane(struct intel_plane *plane,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 3d4bf9b6a0a2..e3339e41ddf7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1183,6 +1183,9 @@  struct intel_plane {
 			   struct intel_plane_state *plane_state);
 	int (*min_cdclk)(const struct intel_crtc_state *crtc_state,
 			 const struct intel_plane_state *plane_state);
+	void (*async_flip)(struct intel_plane *plane,
+			   const struct intel_crtc_state *crtc_state,
+			   const struct intel_plane_state *plane_state);
 };
 
 struct intel_watermark_params {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 76a3d9bfe0de..3634e98b04c1 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -609,6 +609,29 @@  icl_program_input_csc(struct intel_plane *plane,
 			  PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2), 0x0);
 }
 
+static void
+skl_program_async_surface_address(struct intel_plane *plane,
+				  const struct intel_crtc_state *crtc_state,
+				  const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	unsigned long irqflags;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	u32 surf_addr = plane_state->color_plane[0].offset;
+	u32 plane_ctl = plane_state->ctl;
+
+	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+
+	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
+	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
+			  intel_plane_ggtt_offset(plane_state) + surf_addr);
+
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+}
+
 static void
 skl_program_plane(struct intel_plane *plane,
 		  const struct intel_crtc_state *crtc_state,
@@ -3095,6 +3118,7 @@  skl_universal_plane_create(struct drm_i915_private *dev_priv,
 	plane->get_hw_state = skl_plane_get_hw_state;
 	plane->check_plane = skl_plane_check;
 	plane->min_cdclk = skl_plane_min_cdclk;
+	plane->async_flip = skl_program_async_surface_address;
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		formats = icl_get_plane_formats(dev_priv, pipe,