From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: linux-kernel@vger.kernel.org,
Roman Stratiienko <r.stratiienko@gmail.com>,
megous@megous.com
Cc: mripard@kernel.org, wens@csie.org, linux-sunxi@googlegroups.com,
dri-devel@lists.freedesktop.org,
Roman Stratiienko <r.stratiienko@gmail.com>
Subject: Re: [PATCH v4 2/2] drm/sun4i: Use CRTC size instead of primary plane size as mixer frame
Date: Mon, 31 May 2021 19:43:42 +0200 [thread overview]
Message-ID: <2057488.cpEgfWUaUY@kista> (raw)
In-Reply-To: <20210528203036.17999-3-r.stratiienko@gmail.com>
Hi!
General note, you should send Allwinner specific patches to linux-
sunxi@lists.linux.dev too. It's already in MAINTAINERS, but probably it's not
yet propagated in all trees.
Dne petek, 28. maj 2021 ob 22:30:36 CEST je Roman Stratiienko napisal(a):
> Fixes corrupted display picture when primary plane isn't full-screen.
You should expand this a bit more. Most importantly why this fixes a bug? Rule
of thumb - if you used word "fix" in commit message, most of the time you
should add Fixes tag too.
>
> Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
> ---
> drivers/gpu/drm/sun4i/sun8i_mixer.c | 28 ++++++++++++++++++++++++
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> 2 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/
sun8i_mixer.c
> index 5b42cf25cc86..810c731566c0 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -248,6 +248,33 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32
*hw_format)
> return -EINVAL;
> }
>
> +static void sun8i_mode_set(struct sunxi_engine *engine,
> + struct drm_display_mode *mode)
> +{
> + u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
>crtc_vdisplay);
CRTC variants are not appropriate here. These are adjusted for interlacing and
other stuff. This is important during TCON configuration, not here. Just drop
"crtc_" prefix.
Best regards,
Jernej
> + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> + u32 bld_base = sun8i_blender_base(mixer);
> + u32 val;
> +
> + DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> + mode->crtc_hdisplay, mode->crtc_vdisplay);
> + regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> + regmap_write(mixer->engine.regs,
> + SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +
> + if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> + val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> + else
> + val = 0;
> +
> + regmap_update_bits(mixer->engine.regs,
> + SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> + SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> + val);
> + DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> + val ? "on" : "off");
> +}
> +
> static void sun8i_mixer_commit(struct sunxi_engine *engine)
> {
> DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -301,6 +328,7 @@ static struct drm_plane **sun8i_layers_init(struct
drm_device *drm,
> static const struct sunxi_engine_ops sun8i_engine_ops = {
> .commit = sun8i_mixer_commit,
> .layers_init = sun8i_layers_init,
> + .mode_set = sun8i_mode_set,
> };
>
> static const struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/
sun8i_ui_layer.c
> index 0db164a774a1..d66fff582278 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
sun8i_mixer *mixer, int channel,
> insize = SUN8I_MIXER_SIZE(src_w, src_h);
> outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
>
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> - bool interlaced = false;
> - u32 val;
> -
> - DRM_DEBUG_DRIVER("Primary layer, updating global size
W: %u H: %u\n",
> - dst_w, dst_h);
> - regmap_write(mixer->engine.regs,
> - SUN8I_MIXER_GLOBAL_SIZE,
> - outsize);
> - regmap_write(mixer->engine.regs,
> - SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
outsize);
> -
> - if (state->crtc)
> - interlaced = state->crtc->state-
>adjusted_mode.flags
> - & DRM_MODE_FLAG_INTERLACE;
> -
> - if (interlaced)
> - val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> - else
> - val = 0;
> -
> - regmap_update_bits(mixer->engine.regs,
> -
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> -
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> - val);
> -
> - DRM_DEBUG_DRIVER("Switching display mixer interlaced
mode %s\n",
> - interlaced ? "on" : "off");
> - }
> -
> /* Set height and width */
> DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> state->src.x1 >> 16, state->src.y1 >> 16);
>
WARNING: multiple messages have this Message-ID (diff)
From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: linux-kernel@vger.kernel.org,
Roman Stratiienko <r.stratiienko@gmail.com>,
megous@megous.com
Cc: dri-devel@lists.freedesktop.org, wens@csie.org,
linux-sunxi@googlegroups.com,
Roman Stratiienko <r.stratiienko@gmail.com>
Subject: Re: [PATCH v4 2/2] drm/sun4i: Use CRTC size instead of primary plane size as mixer frame
Date: Mon, 31 May 2021 19:43:42 +0200 [thread overview]
Message-ID: <2057488.cpEgfWUaUY@kista> (raw)
In-Reply-To: <20210528203036.17999-3-r.stratiienko@gmail.com>
Hi!
General note, you should send Allwinner specific patches to linux-
sunxi@lists.linux.dev too. It's already in MAINTAINERS, but probably it's not
yet propagated in all trees.
Dne petek, 28. maj 2021 ob 22:30:36 CEST je Roman Stratiienko napisal(a):
> Fixes corrupted display picture when primary plane isn't full-screen.
You should expand this a bit more. Most importantly why this fixes a bug? Rule
of thumb - if you used word "fix" in commit message, most of the time you
should add Fixes tag too.
>
> Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
> ---
> drivers/gpu/drm/sun4i/sun8i_mixer.c | 28 ++++++++++++++++++++++++
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> 2 files changed, 28 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/
sun8i_mixer.c
> index 5b42cf25cc86..810c731566c0 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -248,6 +248,33 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32
*hw_format)
> return -EINVAL;
> }
>
> +static void sun8i_mode_set(struct sunxi_engine *engine,
> + struct drm_display_mode *mode)
> +{
> + u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
>crtc_vdisplay);
CRTC variants are not appropriate here. These are adjusted for interlacing and
other stuff. This is important during TCON configuration, not here. Just drop
"crtc_" prefix.
Best regards,
Jernej
> + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> + u32 bld_base = sun8i_blender_base(mixer);
> + u32 val;
> +
> + DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> + mode->crtc_hdisplay, mode->crtc_vdisplay);
> + regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> + regmap_write(mixer->engine.regs,
> + SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +
> + if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> + val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> + else
> + val = 0;
> +
> + regmap_update_bits(mixer->engine.regs,
> + SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> + SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> + val);
> + DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> + val ? "on" : "off");
> +}
> +
> static void sun8i_mixer_commit(struct sunxi_engine *engine)
> {
> DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -301,6 +328,7 @@ static struct drm_plane **sun8i_layers_init(struct
drm_device *drm,
> static const struct sunxi_engine_ops sun8i_engine_ops = {
> .commit = sun8i_mixer_commit,
> .layers_init = sun8i_layers_init,
> + .mode_set = sun8i_mode_set,
> };
>
> static const struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/
sun8i_ui_layer.c
> index 0db164a774a1..d66fff582278 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
sun8i_mixer *mixer, int channel,
> insize = SUN8I_MIXER_SIZE(src_w, src_h);
> outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
>
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> - bool interlaced = false;
> - u32 val;
> -
> - DRM_DEBUG_DRIVER("Primary layer, updating global size
W: %u H: %u\n",
> - dst_w, dst_h);
> - regmap_write(mixer->engine.regs,
> - SUN8I_MIXER_GLOBAL_SIZE,
> - outsize);
> - regmap_write(mixer->engine.regs,
> - SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
outsize);
> -
> - if (state->crtc)
> - interlaced = state->crtc->state-
>adjusted_mode.flags
> - & DRM_MODE_FLAG_INTERLACE;
> -
> - if (interlaced)
> - val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> - else
> - val = 0;
> -
> - regmap_update_bits(mixer->engine.regs,
> -
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> -
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> - val);
> -
> - DRM_DEBUG_DRIVER("Switching display mixer interlaced
mode %s\n",
> - interlaced ? "on" : "off");
> - }
> -
> /* Set height and width */
> DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> state->src.x1 >> 16, state->src.y1 >> 16);
>
next prev parent reply other threads:[~2021-05-31 17:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-28 20:30 [PATCH v4 0/2] drm/sun4i: Set mixer frame to display size for DE2.0+ Roman Stratiienko
2021-05-28 20:30 ` Roman Stratiienko
2021-05-28 20:30 ` [PATCH v4 1/2] drm/sun4i: Add mode_set callback to the engine Roman Stratiienko
2021-05-28 20:30 ` Roman Stratiienko
2021-05-31 8:38 ` [linux-sunxi] " Priit Laes
2021-05-31 8:38 ` Priit Laes
2021-05-31 17:24 ` Jernej Škrabec
2021-05-31 17:24 ` Jernej Škrabec
2021-05-31 17:48 ` Jernej Škrabec
2021-05-31 17:48 ` Jernej Škrabec
2021-05-28 20:30 ` [PATCH v4 2/2] drm/sun4i: Use CRTC size instead of primary plane size as mixer frame Roman Stratiienko
2021-05-28 20:30 ` Roman Stratiienko
2021-05-28 20:41 ` Roman Stratiienko
2021-05-28 20:41 ` Roman Stratiienko
2021-05-31 17:43 ` Jernej Škrabec [this message]
2021-05-31 17:43 ` Jernej Škrabec
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2057488.cpEgfWUaUY@kista \
--to=jernej.skrabec@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=megous@megous.com \
--cc=mripard@kernel.org \
--cc=r.stratiienko@gmail.com \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.