Thank you Daniel,
Now this works perfectly, all while I was on vacation.
On 08/07/2020 17:20, Daniel Vetter wrote:
> Gives us proper nonblocking support for free, and a pile of other
> things. The tilcdc code is simply old enough that it was never
> converted over, but was stuck forever with the copypasta from when it
> was initially merged.
>
> The riskiest thing with this conversion is maybe that there's an issue
> with the vblank handling or vblank event handling, which will upset
> the modern commit support in atomic helpers. But from a cursory review
> drm_crtc_vblank_on/off is called in the right places, and the event
> handling also seems to exist (albeit with much hand-rolling and
> probably some races, could perhaps be converted over to
> drm_crtc_arm_vblank_event without any real loss).
>
> Motivated by me not having to hand-roll the dma-fence annotations for
> this.
>
> v2: Clear out crtc_state->event when we're handling the event, to
> avoid upsetting the helpers (reported by Jyri).
>
> v3: Also send out even whent the crtc is getting disabled. Tilcdc looks a
> bit like conversion to simple display helpers would work out really
> nice.
>
Probably. Should take a closer looks some day when I have time.
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Jyri Sarha <jsa...@ti.com>
> Cc: Tomi Valkeinen <tomi.valkei...@ti.com>
Tested-by: Jyri Sarha <jsa...@ti.com>
Reviewed-by: Jyri Sarha <jsa...@ti.com>
> --
> From logs looks like we're not stuck when disabling the display, so I
> hacked in a bit of code for that too. Like mentioned above, tilcdc
> looks like a perfect candidate for simple display helpers, I think
> that would simplify a _lot_ of code here.
> -Daniel
> ---
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 13 ++++++++
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 47 +--------------------------
> drivers/gpu/drm/tilcdc/tilcdc_plane.c | 8 +++--
> 3 files changed, 19 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index e9dd5e5cb4e7..1856962411c7 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -537,6 +537,18 @@ static void tilcdc_crtc_atomic_disable(struct drm_crtc
> *crtc,
> tilcdc_crtc_disable(crtc);
> }
>
> +static void tilcdc_crtc_atomic_flush(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> +{
> + if (!crtc->state->event)
> + return;
> +
> + spin_lock_irq(&crtc->dev->event_lock);
> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> + crtc->state->event = NULL;
> + spin_unlock_irq(&crtc->dev->event_lock);
> +}
> +
> void tilcdc_crtc_shutdown(struct drm_crtc *crtc)
> {
> tilcdc_crtc_off(crtc, true);
> @@ -822,6 +834,7 @@ static const struct drm_crtc_helper_funcs
> tilcdc_crtc_helper_funcs = {
> .atomic_check = tilcdc_crtc_atomic_check,
> .atomic_enable = tilcdc_crtc_atomic_enable,
> .atomic_disable = tilcdc_crtc_atomic_disable,
> + .atomic_flush = tilcdc_crtc_atomic_flush,
> };
>
> void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 0d74a6443263..4f5fc3e87383 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -87,55 +87,10 @@ static int tilcdc_atomic_check(struct drm_device *dev,
> return ret;
> }
>
> -static int tilcdc_commit(struct drm_device *dev,
> - struct drm_atomic_state *state,
> - bool async)
> -{
> - int ret;
> -
> - ret = drm_atomic_helper_prepare_planes(dev, state);
> - if (ret)
> - return ret;
> -
> - ret = drm_atomic_helper_swap_state(state, true);
> - if (ret) {
> - drm_atomic_helper_cleanup_planes(dev, state);
> - return ret;
> - }
> -
> - /*
> - * Everything below can be run asynchronously without the need to grab
> - * any modeset locks at all under one condition: It must be guaranteed
> - * that the asynchronous work has either been cancelled (if the driver
> - * supports it, which at least requires that the framebuffers get
> - * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
> - * before the new state gets committed on the software side with
> - * drm_atomic_helper_swap_state().
> - *
> - * This scheme allows new atomic state updates to be prepared and
> - * checked in parallel to the asynchronous completion of the previous
> - * update. Which is important since compositors need to figure out the
> - * composition of the next frame right after having submitted the
> - * current layout.
> - */
> -
> - drm_atomic_helper_commit_modeset_disables(dev, state);
> -
> - drm_atomic_helper_commit_planes(dev, state, 0);
> -
> - drm_atomic_helper_commit_modeset_enables(dev, state);
> -
> - drm_atomic_helper_wait_for_vblanks(dev, state);
> -
> - drm_atomic_helper_cleanup_planes(dev, state);
> -
> - return 0;
> -}
> -
> static const struct drm_mode_config_funcs mode_config_funcs = {
> .fb_create = drm_gem_fb_create,
> .atomic_check = tilcdc_atomic_check,
> - .atomic_commit = tilcdc_commit,
> + .atomic_commit = drm_atomic_helper_commit,
> };
>
> static void modeset_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> index 0d09b31ae759..2f681a713815 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
> @@ -83,9 +83,11 @@ static void tilcdc_plane_atomic_update(struct drm_plane
> *plane,
> if (WARN_ON(!state->fb || !state->crtc->state))
> return;
>
> - tilcdc_crtc_update_fb(state->crtc,
> - state->fb,
> - state->crtc->state->event);
> + if (tilcdc_crtc_update_fb(state->crtc,
> + state->fb,
> + state->crtc->state->event) == 0) {
> + state->crtc->state->event = NULL;
> + }
> }
>
> static const struct drm_plane_helper_funcs plane_helper_funcs = {
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
