Received: by 2002:a05:6a10:2785:0:0:0:0 with SMTP id ia5csp205278pxb; Wed, 13 Jan 2021 01:16:53 -0800 (PST) X-Google-Smtp-Source: ABdhPJzTDZ1lY2xp4/Rf+ks7AF/7g4tmrv2J+diFTlPAjHFXaQnWEIWirFhwmHGS2xe5nu7ntcDS X-Received: by 2002:a17:906:31d2:: with SMTP id f18mr843642ejf.407.1610529413768; Wed, 13 Jan 2021 01:16:53 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1610529413; cv=none; d=google.com; s=arc-20160816; b=gqwaFT0obOw+z4nit8GSiUpZAho0j5hfP0yuKja1qyoyQ/cmUarM/rpD/RsX4nK5ep eEGD242Elkv70Y5STJNzugAUutuqZGl2IbXwbZkFdZeZ0IYORKUL3mhaSN3t01opZNjH ZhKMECfNEPpURPcWNcNMif2jK/aQHSQcUXG03LBmktux12dWBK8QogjR3yv372CZvsBW 19RfAsrkLFSizCuLQJwPoddxvlvkFrdZbNdP6CApJfn7kW8CQKW5HbnR6m9e05KxqXs2 KhwhAXYz+Uv2MgfC6Q8IGO+GhfbHkhB0AkJAQrySIEZiaMflkW67wUb70+xV5DoIg+Mp 8zZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature :dkim-signature; bh=Tn9fTmFYI/uQtrgkhZOhaGqWOASvTLv8TiF5nATR2Ss=; b=TO3cra2ktIES7uBYbnJgXAOtL38e5BVjuntjjrqnVPKQd0Kf7yTjd7dIvgUOTC6Tue z9gJb2tW/vl8BPgZM0BrspmHU3ZS3CWJY5plY9inpRVzEbNSgg54qNmghwg1zgfNOzUs U3oGYJLVO2Fcu7PcfiuH6QBgQsixYudb00+C0k2Y8vkxO2SM0KI356fZFkEajCWZ2Sz3 iZeJHyShdasKlRZ++E6AeFdKDJ4PXFSkPjjZoDYiXxuTUKKqdluhmvwE12+dYtRxhbQw PeK6QSx58moIA3YvOmy3Sxwz7o0/NEBtnPuEAFlJ2PVTHmB3prCoRXtk9z/tiEBJLNim q0LQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@cerno.tech header.s=fm1 header.b=CZrPKwH3; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=D3wCxVaZ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cerno.tech Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id a5si628966ejv.428.2021.01.13.01.16.29; Wed, 13 Jan 2021 01:16:53 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@cerno.tech header.s=fm1 header.b=CZrPKwH3; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=D3wCxVaZ; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cerno.tech Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727475AbhAMJOa (ORCPT + 99 others); Wed, 13 Jan 2021 04:14:30 -0500 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:33117 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727398AbhAMJOa (ORCPT ); Wed, 13 Jan 2021 04:14:30 -0500 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 381185C0194; Wed, 13 Jan 2021 04:13:23 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Wed, 13 Jan 2021 04:13:23 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=Tn9fTmFYI/uQtrgkhZOhaGqWOAS vTLv8TiF5nATR2Ss=; b=CZrPKwH3QSiZepq1UFfaxHTCzECBKR83BxdgoftWeWc +Im6Aw26YlAHS3ZitHwoTtUvwp4DRxrfeho1ALkzj7IvjEuZxPqnky6afNQuwLId ZYiIlgRX6eZBL6HECRgrkGINUFni2AsfyeZsMxTSII0iXPtjk/u81rHqJtTch9bf Txt8YRSuZEiO+qeMMJdgpo1XNHJzHO8cI8/y4If4ynrSvM12R0qFpu0vHNk4p339 MVx+ce1ztx5DEDCUPqXuPNsSxtBfrhm84EYszK18rB0mR86ONBJ335XNnGIIPgK3 04l1JFwQ/fS9jrEne3Raor8yuGLz6rBhDVoENkDFOXQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=Tn9fTm FYI/uQtrgkhZOhaGqWOASvTLv8TiF5nATR2Ss=; b=D3wCxVaZilRDnnL0DUx7SF +gcNeHwEZKywNfjdkFbDugXJ6JoiF36P88cRHvyy+Ukbm/aIrSS9VkaxrWWrWBcP HA1NK+OFbU4lrGzf5q5H+pAe0phm+Te/xreoKr0TpJJ1ldO2JiY9TzWwx0R9JDVo 900UR/WBGrScTbIG1BU9bdPxXuTQwGFP56lrzu9tULX01RqeYj7Vsvg6h6mZO+WP f6v1S7oToFwUNGn4DRSeQ0Qt+zqv8PNCcfuV5aOW7GRZKVQvto+88gimuo9jjg00 f91ZV7dIxiWiOg9ta0b6gO2mBlZ9n8Ihwb4Kso61Of8avCqy2sZ3E35k1iRroJjg == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedukedrtdefgddtudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofgrgihimhgv ucftihhprghrugcuoehmrgigihhmvgestggvrhhnohdrthgvtghhqeenucggtffrrghtth gvrhhnpeelkeeghefhuddtleejgfeljeffheffgfeijefhgfeufefhtdevteegheeiheeg udenucfkphepledtrdekledrieekrdejieenucevlhhushhtvghrufhiiigvpedtnecurf grrhgrmhepmhgrihhlfhhrohhmpehmrgigihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id C04A71080059; Wed, 13 Jan 2021 04:13:21 -0500 (EST) Date: Wed, 13 Jan 2021 10:13:19 +0100 From: Maxime Ripard To: Jernej Skrabec Cc: wens@csie.org, airlied@linux.ie, daniel@ffwll.ch, paul.kocialkowski@bootlin.com, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com, Roman Stratiienko Subject: Re: [PATCH v3] drm/sun4i: de2: Reimplement plane z position setting logic Message-ID: <20210113091319.vcemlo5ldn27472h@gilmour> References: <20210106204630.1800284-1-jernej.skrabec@siol.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7lkz52fdez73ediq" Content-Disposition: inline In-Reply-To: <20210106204630.1800284-1-jernej.skrabec@siol.net> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --7lkz52fdez73ediq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Wed, Jan 06, 2021 at 09:46:30PM +0100, Jernej Skrabec wrote: > From: Roman Stratiienko >=20 > To set blending channel order register software needs to know state and > position of each channel, which impossible at plane commit stage. >=20 > Move this procedure to atomic_flush stage, where all necessary information > is available. Expanding a bit on what the blending order register is supposed to be doing, why it's impossible at the plane commit, and why atomic_flush is a better option would be nice > Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") > Fixes: d8b3f454dab4 ("drm/sun4i: sun8i: Avoid clearing blending order at = each atomic commit") > Signed-off-by: Roman Stratiienko > [rebased, addressed comments] > Signed-off-by: Jernej Skrabec > --- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 57 +++++++++++++++++++++----- > drivers/gpu/drm/sun4i/sun8i_mixer.h | 5 +++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 +++---------------- > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 42 +++---------------- > 4 files changed, 64 insertions(+), 82 deletions(-) >=20 > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/= sun8i_mixer.c > index 5b42cf25cc86..d2153b10b08d 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -250,6 +250,50 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw= _format) > =20 > static void sun8i_mixer_commit(struct sunxi_engine *engine) > { > + struct sun8i_mixer *mixer =3D engine_to_sun8i_mixer(engine); > + int channel_by_zpos[SUN8I_MIXER_MAX_CHANNELS]; > + u32 base =3D sun8i_blender_base(mixer); > + u32 route =3D 0, pipe_ctl =3D 0; > + unsigned int channel_count; > + int i, j; > + > + channel_count =3D mixer->cfg->vi_num + mixer->cfg->ui_num; > + > + DRM_DEBUG_DRIVER("Update blender routing\n"); > + > + for (i =3D 0; i < SUN8I_MIXER_MAX_CHANNELS; i++) > + channel_by_zpos[i] =3D -1; > + > + for (i =3D 0; i < channel_count; i++) { > + int zpos =3D mixer->channel_zpos[i]; Why do we need the channel_zpos in the mixer structure, this looks related to the state itself, so we should store it into a custom state structure > + if (zpos >=3D 0 && zpos < channel_count) > + channel_by_zpos[zpos] =3D i; > + } > + > + j =3D 0; > + for (i =3D 0; i < channel_count; i++) { > + int ch =3D channel_by_zpos[i]; > + > + if (ch >=3D 0) { > + pipe_ctl |=3D SUN8I_MIXER_BLEND_PIPE_CTL_EN(j); > + route |=3D ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > + j++; > + } > + } Similarly, having a comment somewhere to explain that algorithm would be nice. > + /* > + * Set fill color of bottom plane to black. Generally not needed > + * except when VI plane is at bottom (zpos =3D 0) and enabled. > + */ > + pipe_ctl |=3D SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_PIPE_CTL(base), pipe_ctl); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_ROUTE(base), route); > + > DRM_DEBUG_DRIVER("Committing changes\n"); > =20 > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > @@ -479,23 +523,16 @@ static int sun8i_mixer_bind(struct device *dev, str= uct device *master, > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base), > SUN8I_MIXER_BLEND_COLOR_BLACK); > =20 > - /* > - * Set fill color of bottom plane to black. Generally not needed > - * except when VI plane is at bottom (zpos =3D 0) and enabled. > - */ > - regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0)); > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0), > SUN8I_MIXER_BLEND_COLOR_BLACK); > =20 > plane_cnt =3D mixer->cfg->vi_num + mixer->cfg->ui_num; > - for (i =3D 0; i < plane_cnt; i++) > + for (i =3D 0; i < plane_cnt; i++) { > + mixer->channel_zpos[i] =3D -1; > regmap_write(mixer->engine.regs, > SUN8I_MIXER_BLEND_MODE(base, i), > SUN8I_MIXER_BLEND_MODE_DEF); > - > - regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); > + } > =20 > return 0; > =20 > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/= sun8i_mixer.h > index 7576b523fdbb..7b378d6e4dd9 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -12,6 +12,8 @@ > =20 > #include "sunxi_engine.h" > =20 > +#define SUN8I_MIXER_MAX_CHANNELS 5 > + > #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) > #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x)) > =20 > @@ -179,6 +181,9 @@ struct sun8i_mixer { > =20 > struct clk *bus_clk; > struct clk *mod_clk; > + > + /* -1 means that layer is disabled */ > + int channel_zpos[SUN8I_MIXER_MAX_CHANNELS]; > }; > =20 > static inline struct sun8i_mixer * > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun= 4i/sun8i_ui_layer.c > index 816ad4ce8996..9f82e7c33e90 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > @@ -24,12 +24,10 @@ > #include "sun8i_ui_scaler.h" > =20 > static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, > - int overlay, bool enable, unsigned int zpos, > - unsigned int old_zpos) > + int overlay, bool enable, unsigned int zpos) > { > - u32 val, bld_base, ch_base; > + u32 val, ch_base; > =20 > - bld_base =3D sun8i_blender_base(mixer); > ch_base =3D sun8i_channel_base(mixer, channel); > =20 > DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n", > @@ -44,32 +42,7 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *= mixer, int channel, > SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay), > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val); > =20 > - if (!enable || zpos !=3D old_zpos) { > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), > - SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos), > - 0); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), > - 0); > - } > - > - if (enable) { > - val =3D SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), > - val, val); > - > - val =3D channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), > - val); > - } > + mixer->channel_zpos[channel] =3D enable ? zpos : -1; > } > =20 > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int ch= annel, > @@ -267,11 +240,9 @@ static void sun8i_ui_layer_atomic_disable(struct drm= _plane *plane, > struct drm_plane_state *old_state) > { > struct sun8i_ui_layer *layer =3D plane_to_sun8i_ui_layer(plane); > - unsigned int old_zpos =3D old_state->normalized_zpos; > struct sun8i_mixer *mixer =3D layer->mixer; > =20 > - sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0, > - old_zpos); > + sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0); > } > =20 > static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, > @@ -279,12 +250,11 @@ static void sun8i_ui_layer_atomic_update(struct drm= _plane *plane, > { > struct sun8i_ui_layer *layer =3D plane_to_sun8i_ui_layer(plane); > unsigned int zpos =3D plane->state->normalized_zpos; > - unsigned int old_zpos =3D old_state->normalized_zpos; > struct sun8i_mixer *mixer =3D layer->mixer; > =20 > if (!plane->state->visible) { > sun8i_ui_layer_enable(mixer, layer->channel, > - layer->overlay, false, 0, old_zpos); > + layer->overlay, false, 0); > return; > } > =20 > @@ -295,7 +265,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_p= lane *plane, > sun8i_ui_layer_update_buffer(mixer, layer->channel, > layer->overlay, plane); > sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, > - true, zpos, old_zpos); > + true, zpos); > } The prototype change could be in a separate patch, it would make things easier to review. Maxime --7lkz52fdez73ediq Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX/65rwAKCRDj7w1vZxhR xdV4APsFPA4cMgAoq4Sb2HMWf7gmKjULe5gHGRG121MpvmEH5wEA2ceHxq3EltdV mDfAi7tQAjzWj/4nN0MjCfR2kBt1rAY= =0eaD -----END PGP SIGNATURE----- --7lkz52fdez73ediq--