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: linkBe 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.