From: Roman Stratiienko <[email protected]>
Create callback to update engine's registers on mode change.
Signed-off-by: Roman Stratiienko <[email protected]>
Reviewed-by: Jernej Skrabec <[email protected]>
---
v2:
- Split commit in 2 parts.
- Add description to mode_set callback
- Dropped 1 line from sun4i_crtc_mode_set_nofb()
- Add struct drm_display_mode declaration (fix build warning)
v3:
- Pick reviewed-by line
- Add missing 'and' word to the mode_set callback description.
---
drivers/gpu/drm/sun4i/sun4i_crtc.c | 3 +++
drivers/gpu/drm/sun4i/sunxi_engine.h | 12 ++++++++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index 3a153648b369..f9c627d601c3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -141,6 +141,9 @@ static void sun4i_crtc_mode_set_nofb(struct drm_crtc *crtc)
struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
sun4i_tcon_mode_set(scrtc->tcon, encoder, mode);
+
+ if (scrtc->engine->ops->mode_set)
+ scrtc->engine->ops->mode_set(scrtc->engine, mode);
}
static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h b/drivers/gpu/drm/sun4i/sunxi_engine.h
index 548710a936d5..7faa844646ff 100644
--- a/drivers/gpu/drm/sun4i/sunxi_engine.h
+++ b/drivers/gpu/drm/sun4i/sunxi_engine.h
@@ -9,6 +9,7 @@
struct drm_plane;
struct drm_device;
struct drm_crtc_state;
+struct drm_display_mode;
struct sunxi_engine;
@@ -108,6 +109,17 @@ struct sunxi_engine_ops {
* This function is optional.
*/
void (*vblank_quirk)(struct sunxi_engine *engine);
+
+ /**
+ * @mode_set:
+ *
+ * This callback is used to update engine registers that
+ * responsible for display frame size and other mode attributes.
+ *
+ * This function is optional.
+ */
+ void (*mode_set)(struct sunxi_engine *engine,
+ struct drm_display_mode *mode);
};
/**
--
2.17.1
From: Roman Stratiienko <[email protected]>
According to DRM documentation the only difference between PRIMARY
and OVERLAY plane is that each CRTC must have PRIMARY plane and
OVERLAY are optional.
Allow PRIMARY plane to have dimension different from full-screen.
Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
Signed-off-by: Roman Stratiienko <[email protected]>
---
v2:
- Split commit in 2 parts
- Add Fixes line to the commit message
v3:
- Address review comments of v2 + removed 3 local varibles
- Change 'Fixes' line
Since I've put more changes from my side, please review/sign again.
---
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 8b803eb903b8..658cf442c121 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format)
return NULL;
}
+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);
+ 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");
@@ -310,6 +337,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 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 4343ea9f8cf8..f01ac55191f1 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);
--
2.17.1
Hi!
Dne sreda, 01. januar 2020 ob 21:47:50 CET je
[email protected] napisal(a):
> From: Roman Stratiienko <[email protected]>
>
> According to DRM documentation the only difference between PRIMARY
> and OVERLAY plane is that each CRTC must have PRIMARY plane and
> OVERLAY are optional.
>
> Allow PRIMARY plane to have dimension different from full-screen.
>
> Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> Signed-off-by: Roman Stratiienko <[email protected]>
This looks great now.
Reviewed-by: Jernej Skrabec <[email protected]>
What happened to other patches in the series? It would be nice to have a cover
letter for such cases, where you can explain reasons for dropped patches.
Best regards,
Jernej
> ---
> v2:
> - Split commit in 2 parts
> - Add Fixes line to the commit message
>
> v3:
> - Address review comments of v2 + removed 3 local varibles
> - Change 'Fixes' line
>
> Since I've put more changes from my side, please review/sign again.
> ---
> 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 8b803eb903b8..658cf442c121
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> format) return NULL;
> }
>
> +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);
> + 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");
> @@ -310,6 +337,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 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 4343ea9f8cf8..f01ac55191f1
> 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);
Hi Roman,
Your domain has DMARC setup with the "reject" policy.
This means emails from your domain may be subsequently rejected by people
using email forwarders (such as @kernel.org) to forward to Gmail.
I suggest using another email address to send patches, or ask your IT
people to drop the policy to "quarantine", which makes the email go to
the SPAM folder instead of outright rejecting them.
ChenYu
On Thu, Jan 2, 2020 at 3:43 PM Jernej Škrabec <[email protected]> wrote:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> [email protected] napisal(a):
> > From: Roman Stratiienko <[email protected]>
> >
> > According to DRM documentation the only difference between PRIMARY
> > and OVERLAY plane is that each CRTC must have PRIMARY plane and
> > OVERLAY are optional.
> >
> > Allow PRIMARY plane to have dimension different from full-screen.
> >
> > Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> > Signed-off-by: Roman Stratiienko <[email protected]>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <[email protected]>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> > 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 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> > }
> >
> > +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);
> > + 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");
> > @@ -310,6 +337,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 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 4343ea9f8cf8..f01ac55191f1
> > 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);
>
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi,
On Wed, Jan 01, 2020 at 10:47:50PM +0200, [email protected] wrote:
> From: Roman Stratiienko <[email protected]>
>
> According to DRM documentation the only difference between PRIMARY
> and OVERLAY plane is that each CRTC must have PRIMARY plane and
> OVERLAY are optional.
>
> Allow PRIMARY plane to have dimension different from full-screen.
>
> Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> Signed-off-by: Roman Stratiienko <[email protected]>
So it applies to all the 4 patches you've sent, but this lacks some
context.
There's a few questions that should be answered here:
- Which situation is it fixing?
- What tool / userspace stack is it fixing?
- What happens with your fix? Do you set the plane at coordinates
0,0 (meaning you'll crop the top-lef corner), do you center it? If
the plane is smaller than the CTRC size, what is set on the edges?
Thanks!
Maxime
чт, 2 янв. 2020 г., 12:08 Maxime Ripard <[email protected]>:
>
> Hi,
>
> On Wed, Jan 01, 2020 at 10:47:50PM +0200, [email protected] wrote:
> > From: Roman Stratiienko <[email protected]>
> >
> > According to DRM documentation the only difference between PRIMARY
> > and OVERLAY plane is that each CRTC must have PRIMARY plane and
> > OVERLAY are optional.
> >
> > Allow PRIMARY plane to have dimension different from full-screen.
> >
> > Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> > Signed-off-by: Roman Stratiienko <[email protected]>
>
> So it applies to all the 4 patches you've sent, but this lacks some
> context.
>
> There's a few questions that should be answered here:
> - Which situation is it fixing?
Setting primary plane size less than crtc breaks composition. Also
shifting top left corner also breaks it.
> - What tool / userspace stack is it fixing?
I am using Android userspace and drm_hwcomposer HAL.
@Jernej, you've said that you observed similar issue. Could you share
what userspace have you used?
> - What happens with your fix? Do you set the plane at coordinates
> 0,0 (meaning you'll crop the top-lef corner), do you center it? If
> the plane is smaller than the CTRC size, what is set on the edges?
You can put primary plane to any part of the screen and make it as
small as 8x8 (according to the datasheet) . Background would be filled
with black color, that is default, but it also could be overridden by
setting corresponding registers.
>
>
> Thanks!
> Maxime
чт, 2 янв. 2020 г., 09:42 Jernej Škrabec <[email protected]>:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> [email protected] napisal(a):
> > From: Roman Stratiienko <[email protected]>
> >
> > According to DRM documentation the only difference between PRIMARY
> > and OVERLAY plane is that each CRTC must have PRIMARY plane and
> > OVERLAY are optional.
> >
> > Allow PRIMARY plane to have dimension different from full-screen.
> >
> > Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> > Signed-off-by: Roman Stratiienko <[email protected]>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <[email protected]>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
>
>
>
Thanks and sorry for any mistakes in procedure, I'll try to follow the
rules in the future..
Some of commits requires more time to test/deliver than others. So
splitting it into smaller chunks helps to deliver them earlier.
>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> > 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 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> > }
> >
> > +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);
> > + 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");
> > @@ -310,6 +337,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 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 4343ea9f8cf8..f01ac55191f1
> > 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);
>
>
>
>
Hi!
Dne četrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> чт, 2 янв. 2020 г., 12:08 Maxime Ripard <[email protected]>:
> > Hi,
> >
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200,
[email protected] wrote:
> > > From: Roman Stratiienko <[email protected]>
> > >
> > > According to DRM documentation the only difference between PRIMARY
> > > and OVERLAY plane is that each CRTC must have PRIMARY plane and
> > > OVERLAY are optional.
> > >
> > > Allow PRIMARY plane to have dimension different from full-screen.
> > >
> > > Fixes: 5bb5f5dafa1a ("drm/sun4i: Reorganize UI layer code in DE2")
> > > Signed-off-by: Roman Stratiienko <[email protected]>
> >
> > So it applies to all the 4 patches you've sent, but this lacks some
> > context.
> >
> > There's a few questions that should be answered here:
> > - Which situation is it fixing?
>
> Setting primary plane size less than crtc breaks composition. Also
> shifting top left corner also breaks it.
True, HW doesn't have notion of primary plane. It's just one plane which is
marked as primary, but otherwise it has same capabilities as others, like x,y
coordinates, size, etc.
>
> > - What tool / userspace stack is it fixing?
>
> I am using Android userspace and drm_hwcomposer HAL.
>
> @Jernej, you've said that you observed similar issue. Could you share
> what userspace have you used?
I observed it with DE1, but it has exactly the same issue. I noticed this
problem on Kodi (gbm version). Kodi first searches for plane capable of
displaying NV12 format (for video) and after that a plane which is capable of
displaying RGB888 format (for GUI). In DE1 case, first plane is primary and
also capable of displaying NV12 format. So when video is displayed which
doesn't cover whole screen, display output is corrupted. However, with such
fix, video playback is correct. Luc Verhaegen make equivalent fix for DE1, where
he also claims primary plane doesn't have to be same size as CRTC output:
https://github.com/libv/fosdem-video-linux/commit/
ae3215d37ca2a55642bcae6c83c3612e26275711
>
> > - What happens with your fix? Do you set the plane at coordinates
> >
> > 0,0 (meaning you'll crop the top-lef corner), do you center it? If
> > the plane is smaller than the CTRC size, what is set on the edges?
>
> You can put primary plane to any part of the screen and make it as
> small as 8x8 (according to the datasheet) . Background would be filled
> with black color, that is default, but it also could be overridden by
> setting corresponding registers.
Correct, same logic as for overlay planes applies.
Best regards,
Jernej
>
> > Thanks!
> > Maxime