Re: [PATCH v2 5/7] drm/vc4: hvs: Move the dlist setup to its own function

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Maxime Ripard <maxime@cerno.tech>, dri-devel@lists.freedesktop.org
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH v2 5/7] drm/vc4: hvs: Move the dlist setup to its own function
Date: Wed, 6 Apr 2022 12:41:14 +0200	[thread overview]
Message-ID: <4ae0c172-52f1-9e03-b150-7657eef331f7@suse.de> (raw)
In-Reply-To: <20220331143744.777652-6-maxime@cerno.tech>

[-- Attachment #1.1: Type: text/plain, Size: 2924 bytes --]

Hi

Am 31.03.22 um 16:37 schrieb Maxime Ripard:
> The vc4_hvs_update_dlist function mostly deals with setting up the
> vblank events and setting up the dlist entry pointer to our current
> active one.
> 
> We'll want to do the former separately from the vblank handling in later
> patches, so let's move it to a function of its own.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/vc4/vc4_hvs.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 71aa5081eaa3..2d540fc11357 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -382,10 +382,19 @@ int vc4_hvs_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
>   	return 0;
>   }
>   
> +static void vc4_hvs_install_dlist(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct vc4_dev *vc4 = to_vc4_dev(dev);
> +	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
> +
> +	HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
> +		  vc4_state->mm.start);
> +}
> +
>   static void vc4_hvs_update_dlist(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
> -	struct vc4_dev *vc4 = to_vc4_dev(dev);
>   	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>   	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
>   	unsigned long flags;
> @@ -405,9 +414,6 @@ static void vc4_hvs_update_dlist(struct drm_crtc *crtc)
>   		spin_unlock_irqrestore(&dev->event_lock, flags);
>   	}
>   
> -	HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel),
> -		  vc4_state->mm.start);
> -
>   	spin_lock_irqsave(&vc4_crtc->irq_lock, flags);
>   	vc4_crtc->current_dlist = vc4_state->mm.start;
>   	spin_unlock_irqrestore(&vc4_crtc->irq_lock, flags);
> @@ -434,6 +440,7 @@ void vc4_hvs_atomic_enable(struct drm_crtc *crtc,
>   	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>   	bool oneshot = vc4_crtc->feeds_txp;
>   
> +	vc4_hvs_install_dlist(crtc);
>   	vc4_hvs_update_dlist(crtc);
>   	vc4_hvs_init_channel(vc4, crtc, mode, oneshot);
>   }
> @@ -509,8 +516,10 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
>   	 * If the CRTC is being disabled, there's no point in updating this
>   	 * information.
>   	 */
> -	if (crtc->state->active && old_state->active)
> +	if (crtc->state->active && old_state->active) {
> +		vc4_hvs_install_dlist(crtc);
>   		vc4_hvs_update_dlist(crtc);
> +	}

No need for braces here.

Best regards
Thomas

>   
>   	if (crtc->state->color_mgmt_changed) {
>   		u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(channel));

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]