[PATCH 1/9] drm/tegra: hub: Remove bogus connection mutex check

[Daniel Vetter]

On Thu, Nov 28, 2019 at 04:37:33PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding at nvidia.com>
> 
> I have no recollection why that check is there, but it seems to trigger
> all the time, so remove it. Everything works fine without.
> 
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
>  drivers/gpu/drm/tegra/hub.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> index 6aca0fd5a8e5..e56c0f7d3a13 100644
> --- a/drivers/gpu/drm/tegra/hub.c
> +++ b/drivers/gpu/drm/tegra/hub.c
> @@ -615,11 +615,8 @@ static struct tegra_display_hub_state *
>  tegra_display_hub_get_state(struct tegra_display_hub *hub,
>  			    struct drm_atomic_state *state)
>  {
> -	struct drm_device *drm = dev_get_drvdata(hub->client.parent);
>  	struct drm_private_state *priv;
>  
> -	WARN_ON(!drm_modeset_is_locked(&drm->mode_config.connection_mutex));

I suspect copypasta from the mst private state stuff, which relied on this
lock to protect it. Except your code never bothered to grab that lock (or
any other) so was technically broken until we added generic locking in

commit b962a12050a387e4bbf3a48745afe1d29d396b0d
Author: Rob Clark <robdclark at gmail.com>
Date:   Mon Oct 22 14:31:22 2018 +0200

    drm/atomic: integrate modeset lock with private objects

Hence this is now ok to drop, originally it wasnt.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Aside: You're single-thread all your atomic updates on the hub->lock,
which might not be what you want. At least updates to separate crtc should
go through in parallel. Usual way to fix this is to add a
tegra_crtc_state->hub_changed that your earlier code sets, and then you
walk the crtc states in the atomic commit (only those, not all, otherwise
you just rebuild that global lock again), and then only grab the hub state
when you need to update something.

Cheers, Daniel

> -
>  	priv = drm_atomic_get_private_obj_state(state, &hub->base);
>  	if (IS_ERR(priv))
>  		return ERR_CAST(priv);
> -- 
> 2.23.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch