Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> writes:
> During an atomic commit, the HVS is configured with a display list
> for the channel matching the associated CRTC. The Pixel Valve (CRTC)
> and encoder are also configured for the new setup at that time.
> While the Pixel Valve and encoder are reconfigured synchronously, the
> HVS is only reconfigured after the display list address (DISPLIST) has
> been updated to the current display list address (DISPLACTX), which is
> the responsibility of the hardware.
>
> The time frame during which the HVS is still running on its previous
> configuration but the CRTC and encoder have been reconfigured already
> can lead to a number of synchronization issues. They will eventually
> cause errors reported on the FIFOs, such as underruns.
>
> With underrun detection enabled (from Boris Brezillon's series), this
> leads to unreliable underrun detection with random false positives.
>
> To ensure a coherent state, wait for each enabled channel of the HVS
> to synchronize its current display list address. This fixes the issue
> of random underrun reporting on commits.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/vc4/vc4_drv.h | 1 +
> drivers/gpu/drm/vc4/vc4_hvs.c | 17 +++++++++++++++++
> drivers/gpu/drm/vc4/vc4_kms.c | 2 ++
> drivers/gpu/drm/vc4/vc4_regs.h | 2 ++
> 4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index c24b078f0593..955f157f5ad0 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -772,6 +772,7 @@ void vc4_irq_reset(struct drm_device *dev);
> extern struct platform_driver vc4_hvs_driver;
> void vc4_hvs_dump_state(struct drm_device *dev);
> int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
> +void vc4_hvs_sync_dlist(struct drm_device *dev);
>
> /* vc4_kms.c */
> int vc4_kms_load(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 5d8c749c9749..1ba60b8e0c2d 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -166,6 +166,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs,
> return 0;
> }
>
> +void vc4_hvs_sync_dlist(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
> + if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
> + continue;
> +
> + ret = wait_for(HVS_READ(SCALER_DISPLACTX(i)) ==
> + HVS_READ(SCALER_DISPLISTX(i)), 1000);
> + WARN(ret, "Timeout waiting for channel %d display list sync\n",
> + i);
> + }
> +}
> +
> static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
> {
> struct platform_device *pdev = to_platform_device(dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 0490edb192a1..2d66a2b57a91 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -155,6 +155,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
>
> drm_atomic_helper_commit_hw_done(state);
>
> + vc4_hvs_sync_dlist(dev);
> +
> drm_atomic_helper_wait_for_flip_done(dev, state);
wait_for_flip_done should already be waiting for DISPLACTX to match our
crtc's display list, though, right (see vc4_crtc_handle_page_flip())?
This seems like a no-op to me.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
