Received-SPF: pass (google.com: domain of linux-kernel+bounces-77295-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Jernej =?utf-8?B?xaBrcmFiZWM=?= <jernej.skrabec@gmail.com>
To: =?utf-8?B?T25kxZllag==?= Jirman <megi@xff.cz>,
 Maxime Ripard <mripard@kernel.org>
Cc: Chen-Yu Tsai <wens@csie.org>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>,
 Daniel Vetter <daniel@ffwll.ch>, Samuel Holland <samuel@sholland.org>,
 dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org,
 linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] drm/sun4i: Fix layer zpos change/atomic modesetting
Date: Thu, 22 Feb 2024 21:02:53 +0100
Message-ID: <2448947.jE0xQCEvom@jernej-laptop>
In-Reply-To: <inuhwnlexpt6dpre4uailtvytjhms4uqeerzehbntczurhcxol@fc4nvkdwffdd>
References:
 <20240216190430.1374132-1-megi@xff.cz> <20240216190430.1374132-4-megi@xff.cz>
 <inuhwnlexpt6dpre4uailtvytjhms4uqeerzehbntczurhcxol@fc4nvkdwffdd>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"

Dne sreda, 21. februar 2024 ob 14:45:20 CET je Maxime Ripard napisal(a):
> Hi,
>=20
> On Fri, Feb 16, 2024 at 08:04:26PM +0100, Ond=C5=99ej Jirman wrote:
> > From: Ondrej Jirman <megi@xff.cz>
> >=20
> > Identical configurations of planes can lead to different (and wrong)
> > layer -> pipe routing at HW level, depending on the order of atomic
> > plane changes.
> >=20
> > For example:
> >=20
> > - Layer 1 is configured to zpos 0 and thus uses pipe 0. No other layer
> >   is enabled. This is a typical situation at boot.
> >=20
> > - When a compositor takes over and layer 3 is enabled,
> >   sun8i_ui_layer_enable() will get called with old_zpos=3D0 zpos=3D1, w=
hich
> >   will lead to incorrect disabling of pipe 0 and enabling of pipe 1.
> >=20
> > What happens is that sun8i_ui_layer_enable() function may disable
> > blender pipes even if it is no longer assigned to its layer.
> >=20
> > To correct this, move the routing setup out of individual plane's
> > atomic_update into crtc's atomic_update, where it can be calculated
> > and updated all at once.
> >=20
> > Remove the atomic_disable callback because it is no longer needed.
> >=20
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
>=20
> I don't have enough knowledge about the mixers code to comment on your
> patch, so I'll let Jernej review it. However, this feels to me like the
> pipe assignment is typically the sort of things that should be dealt
> with device-wide, and in atomic_check.

In DE2 and DE3.0, you cannot move planes between mixers (crtcs), because ea=
ch
one is hardwired to specific mixer. Movable planes are the feature of DE3.3
and one of the pain points for upstreaming the code. Anyway, this commit on=
ly
addresses current issue of enabling and disabling planes and handling zpos.

In atomic check you can only precalculate final register values, but I don't
see any benefit doing that. I think that this code elegantly solves current
issue of enabling or disabling wrong plane in certain situations, so:

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Note, if there is new revision, please rewrite blender regmap_update_bits()
to regmap_write(). Since there is HW issue with reads, I would like to
get rid of regmap_update_bits() calls eventually.

Best regards,
Jernej

>=20
> If I'm talking non-sense, it would be great to mention at least why that
> can't be an option in the commit log.
>=20
> Maxime
>=20