Re: [Intel-gfx] [PATCH v6 5/7] drm/i915: Add dedicated plane hook for async flip case

[Karthik B S <karthik.b.s@intel.com>]

On 9/1/2020 4:57 PM, Ville Syrjälä wrote:
> On Fri, Aug 07, 2020 at 03:05:49PM +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.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_sprite.c | 25 +++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
>> index 2b2d96c59d7f..1c03546a4d2a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>> @@ -609,6 +609,24 @@ 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 drm_i915_private *dev_priv,
>> +				  const struct intel_plane_state *plane_state,
>> +				  enum pipe pipe, enum plane_id plane_id,
>> +				  u32 surf_addr)
>> +{
>> +	unsigned long irqflags;
>> +	u32 plane_ctl = plane_state->ctl;
> 
> Need the bits from skl_plane_ctl_crtc() too.

Thanks for the review.
Sure, I'll update this.
> 
>> +
>> +	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,
>> @@ -637,6 +655,13 @@ skl_program_plane(struct intel_plane *plane,
>>   	u32 keymsk, keymax;
>>   	u32 plane_ctl = plane_state->ctl;
>>   
>> +	/* During Async flip, no other updates are allowed */
>> +	if (crtc_state->uapi.async_flip) {
>> +		skl_program_async_surface_address(dev_priv, plane_state,
>> +						  pipe, plane_id, surf_addr);
>> +		return;
>> +	}
> 
> I'd suggest adding a vfunc for this. Should be able to call it from
> intel_update_plane(). That way we don't need to patch it into each
> and every .update_plane() implementation.
> 

Sure. I will add a vfunc for this in intel_plane and call it directly 
from intel_update_plane()

Thanks,
Karthik.B.S
> 
>> +
>>   	plane_ctl |= skl_plane_ctl_crtc(crtc_state);
>>   
>>   	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>> -- 
>> 2.22.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx