Re: [PATCH 1/2] drm/msm: dpu: Only check flush register against pending flushes

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-09-19 11:56, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> There exists a case where a flush of a plane/dma may have been 
> triggered
> & started from an async commit. If that plane/dma is subsequently 
> disabled
> by the next commit, the flush register will continue to hold the flush
> bit for the disabled plane. Since the bit remains active,
> pending_kickoff_cnt will never decrement and we'll miss frame_done
> events.
> 
I suppose this is the vblank in between the async commit and the next 
commit (one where
the plane is disabled).

If this vblank had consumed the flush bits, it means the HW
has read the configuration and it should have cleared bits.

If you still see the flush bit active, it means the async commit has 
missed the VBLANK boundary
and the HW has not yet taken the cursor configuration. So you are not 
supposed to
get frame_done event.

Comments outside the scope of this patch: To support async and sync 
updates
on the same display commit thread, we should be adding more protection 
to support
concurrency scenarios to avoid more than one ctl flushes per VBLANK 
period.

Thanks,
Jeykumar S.



> This patch limits the check of flush_register to include only those 
> bits
> which have been updated with the latest commit.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 84de385a9f62..60f146f02b77 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -331,7 +331,7 @@ static void dpu_encoder_phys_vid_vblank_irq(void 
> *arg,
> int irq_idx)
>  	if (hw_ctl && hw_ctl->ops.get_flush_register)
>  		flush_register = hw_ctl->ops.get_flush_register(hw_ctl);
> 
> -	if (flush_register == 0)
> +	if (!(flush_register & hw_ctl->ops.get_pending_flush(hw_ctl)))
>  		new_cnt =
> atomic_add_unless(&phys_enc->pending_kickoff_cnt,
>  				-1, 0);
>  	spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno