Re: [PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3

2022-06-08 Thread Roman Stratiienko
ср, 8 июн. 2022 г. в 11:17, Maxime Ripard :
>
> On Mon, Jun 06, 2022 at 01:16:06PM +0300, Roman Stratiienko wrote:
> > вс, 5 июн. 2022 г. в 23:23, Jernej Škrabec :
> > >
> > > Dne nedelja, 05. junij 2022 ob 11:40:18 CEST je Roman Stratiienko 
> > > napisal(a):
> > > > Otherwise alpha value is discarded, resulting incorrect pixel
> > > > apperance on the display.
> > > >
> > > > This also fixes missing transparency for the most bottom layer.
> > >
> > > Can you explain that a bit more?
> >
> > Well... I would recommend reading Bartosz Ciechanowski's blog
> > https://ciechanow.ski/alpha-compositing/ or the Porter-Duff's 1984
> > whitepaper itself.
> >
> > HINT: That magic numbers from sun8i_mixer.h ( 0x03010301 ) corresponds
> > to SOURCE OVER mode.
> >
> > As you can see from the blending equation it outputs both pixel value
> > and alpha value (non-premultiplied data mode).
> >
> > Also single-layer non-premultiplied buffers may have for example
> > (R255,G255,B255,A2) pixel value, which should be sent as {R2, G2, B2}
> > through the physical display interface.
> >
> > When OUTCTL.PREMULTI disabled pixel, the RGB values passes as is, and
> > even 100% transparent data {R255, G255, B255, A0} will appear as 100%
> > opaque white.
>
> Without going into the full explanation about what alpha is, your commit
> log must describe what the bug is exactly, and most importantly how do
> you trigger it.

I do not understand what you want me to add. I checked alpha
appearance manually by
preparing framebuffers with data and presenting it on the display in
various combinations.

I attached the videos and tests as a proof. If you don't believe me
you can always check.

If you find something missing in the commit message or don't like to
see external links feel
free to amend it. From my point of view the patch is complete.

>
> Maxime

Roman


Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property

2022-06-06 Thread Roman Stratiienko
Hi Maxime,

пн, 6 июн. 2022 г. в 12:22, Maxime Ripard :
>
> On Mon, Jun 06, 2022 at 11:20:06AM +0200, Maxime Ripard wrote:
> > On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote:
> > > Hello Jernej,
> > >
> > > Thank you for having a look.
> > >
> > > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec :
> > > >
> > > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko 
> > > > napisal(a):
> > > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > > > "None", "Pre-multiplied", "Coverage"
> > > > >
> > > > > Add the blend mode property and route it to the appropriate registers.
> > > > >
> > > > > Note:
> > > > > "force_premulti" parameter was added to handle multi-overlay channel
> > > > > cases in future changes. It must be set to true for cases when more
> > > > > than 1 overlay layer is used within a channel and at least one of the
> > > > > overlay layers within a group uses premultiplied blending mode.
> > > >
> > > > Please remove this parameter. It's nothing special, so it can be easily 
> > > > added
> > > > once it's actually needed. For now, it only complicates code.
> > >
> > > I would prefer keeping it if you do not have any strong opinion against 
> > > it.
> > >
> > > I am working now on exposing all overlays, so it will be needed soon 
> > > anyway.
> >
> > KMS assumes pre-multiplied alpha anyway, so we shouldn't need a
> > parameter to force it: we're always going to force it.
>
> My bad, I got confused with your other patch.
>
> Still, I agree with Jernej, if it's not needed it only complicates the
> code for no particular reason. If you need it at some point in the
> future, add it then. Otherwise, it's hard to reason about, since we
> don't know what are the expectations that those future patches will
> bring.

Well, existing code already has some sort of support for the sub-overlays:
For example 'int overlay, ' parameter is always 0, but it still passed through
the call stack. Therefore this patch is just aligned with already existing
traditions to keep sub-overlays in mind.

>
> Maxime

Regards,
Roman


Re: [PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3

2022-06-06 Thread Roman Stratiienko
Hi,

вс, 5 июн. 2022 г. в 23:23, Jernej Škrabec :
>
> Dne nedelja, 05. junij 2022 ob 11:40:18 CEST je Roman Stratiienko napisal(a):
> > Otherwise alpha value is discarded, resulting incorrect pixel
> > apperance on the display.
> >
> > This also fixes missing transparency for the most bottom layer.
>
> Can you explain that a bit more?

Well... I would recommend reading Bartosz Ciechanowski's blog
https://ciechanow.ski/alpha-compositing/ or the Porter-Duff's 1984
whitepaper itself.

HINT: That magic numbers from sun8i_mixer.h ( 0x03010301 ) corresponds
to SOURCE OVER mode.

As you can see from the blending equation it outputs both pixel value
and alpha value (non-premultiplied data mode).

Also single-layer non-premultiplied buffers may have for example
(R255,G255,B255,A2) pixel value, which should be sent as {R2, G2, B2}
through the physical display interface.

When OUTCTL.PREMULTI disabled pixel, the RGB values passes as is, and
even 100% transparent data {R255, G255, B255, A0} will appear as 100%
opaque white.

>Also, BSP driver never enables this bit. What are we doing differently?

Are you sure the BSP does not have an issues with presenting
transparent buffers?
Does the sunxi even have a customer-feedback mechanism and publicly
available stable BSP with all the fixes?

Regards,
Roman

>
> >
> > Test applications and videos w/ w/o this patch are available at [1].
> >
> > [1]: https://github.com/GloDroid/glodroid_tests/issues/1
>
> As stated in other emails, commit messages should not contain external links
> (per patch rules).
>
> Best regards,
> Jernej
>
> > Signed-off-by: Roman Stratiienko 
> >
> > ---
> > Changelog:
> >
> > V2: Added code hunk missing in v1
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c | 5 +++--
> >  drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 +
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 6b1711a9a71f..ba2932aaed08
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -320,8 +320,9 @@ static void sun8i_mixer_mode_set(struct sunxi_engine
> > *engine, else
> >   val = 0;
> >
> > - regmap_update_bits(engine->regs,
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> val);
> > + val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY;
> > +
> > + regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> val);
> >
> >   DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> >interlaced ? "on" : "off");
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..bc12c95af6f3
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -65,6 +65,7 @@
> >  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)  (0xf << ((n) << 2))
> >  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2)
> >
> > +#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY BIT(0)
> >  #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED  BIT(1)
> >
> >  #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
>
>
>
>


Re: [PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property

2022-06-06 Thread Roman Stratiienko
Hello Jernej,

Thank you for having a look.

вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec :
>
> Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > "None", "Pre-multiplied", "Coverage"
> >
> > Add the blend mode property and route it to the appropriate registers.
> >
> > Note:
> > "force_premulti" parameter was added to handle multi-overlay channel
> > cases in future changes. It must be set to true for cases when more
> > than 1 overlay layer is used within a channel and at least one of the
> > overlay layers within a group uses premultiplied blending mode.
>
> Please remove this parameter. It's nothing special, so it can be easily added
> once it's actually needed. For now, it only complicates code.

I would prefer keeping it if you do not have any strong opinion against it.

I am working now on exposing all overlays, so it will be needed soon anyway.

Also it helps to better understand the COV2PREMULT mode which has not
the best description in the datasheet. Only after testing this
register using devmem I became confident on its purpose.

>
> >
> > Test:
> > Manually tested all the modes using kmsxx python wrapper with and
> > without 'force_premulti' flag enabled.
> >
> > Signed-off-by: Roman Stratiienko 
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 ++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 -
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +++
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  5 +++
> >  5 files changed, 94 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -65,6 +65,8 @@
> >  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)  (0xf << ((n) << 2))
> >  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)((n) << 2)
> >
> > +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe)   BIT(pipe)
> > +
> >  #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED  BIT(1)
> >
> >  #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)BIT(ch)
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 6ccbbca3176d..29c0d9cca19a
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer
> > *mixer, int channel, }
> >
> >  static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int
> > channel, -int overlay, struct
> drm_plane *plane)
> > + int overlay, struct
> drm_plane *plane,
> > + unsigned int zpos,
> bool force_premulti)
> >  {
> > - u32 mask, val, ch_base;
> > + u32 mask, val, ch_base, bld_base;
> > + bool in_premulti, out_premulti;
> >
> > + bld_base = sun8i_blender_base(mixer);
> >   ch_base = sun8i_channel_base(mixer, channel);
> >
> > + in_premulti = plane->state->pixel_blend_mode ==
> DRM_MODE_BLEND_PREMULTI;
> > + out_premulti = force_premulti || in_premulti;
> > +
> >   mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK;
> >
> >   val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >>
> 8);
> >
> > - val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > + if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
> > + } else {
> > + val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE)
> ?
> > +
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> > +
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> > +
> > + if (in_premulti)
> > + val |=
> SUN8I_MIXER_CHAN_U

[PATCH] drm/sun4i: sun8i: Add support for pixel blend mode property

2022-06-05 Thread Roman Stratiienko
Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
"None", "Pre-multiplied", "Coverage"

Add the blend mode property and route it to the appropriate registers.

Note:
"force_premulti" parameter was added to handle multi-overlay channel
cases in future changes. It must be set to true for cases when more
than 1 overlay layer is used within a channel and at least one of the
overlay layers within a group uses premultiplied blending mode.

Test:
Manually tested all the modes using kmsxx python wrapper with and
without 'force_premulti' flag enabled.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 -
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +++
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  5 +++
 5 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index ebfc276b2464..5c05907e26fb 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -65,6 +65,8 @@
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2))
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)  ((n) << 2)
 
+#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe)
+
 #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1)
 
 #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)  BIT(ch)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 6ccbbca3176d..29c0d9cca19a 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -58,24 +58,46 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer 
*mixer, int channel,
 }
 
 static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
-   int overlay, struct drm_plane *plane)
+   int overlay, struct drm_plane *plane,
+   unsigned int zpos, bool force_premulti)
 {
-   u32 mask, val, ch_base;
+   u32 mask, val, ch_base, bld_base;
+   bool in_premulti, out_premulti;
 
+   bld_base = sun8i_blender_base(mixer);
ch_base = sun8i_channel_base(mixer, channel);
 
+   in_premulti = plane->state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI;
+   out_premulti = force_premulti || in_premulti;
+
mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
-   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK;
 
val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
 
-   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
-   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
-   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+   if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+   val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
+   } else {
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+  
SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   if (in_premulti)
+   val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
+   }
+
+   if (!in_premulti && out_premulti)
+   val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
 
regmap_update_bits(mixer->engine.regs,
   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
   mask, val);
+
+   regmap_update_bits(
+   mixer->engine.regs, SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
+   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
+   out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
 }
 
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -274,7 +296,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
sun8i_ui_layer_update_alpha(mixer, layer->channel,
-   layer->overlay, plane);
+   layer->overlay, plane, zpos, false);
sun8i_ui_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -332,8 +354,8 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
 {
enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;

[PATCH v2] drm/sun4i: Enable output signal premultiplication for DE2/DE3

2022-06-05 Thread Roman Stratiienko
Otherwise alpha value is discarded, resulting incorrect pixel
apperance on the display.

This also fixes missing transparency for the most bottom layer.

Test applications and videos w/ w/o this patch are available at [1].

[1]: https://github.com/GloDroid/glodroid_tests/issues/1
Signed-off-by: Roman Stratiienko 

---
Changelog:

V2: Added code hunk missing in v1
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 5 +++--
 drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 6b1711a9a71f..ba2932aaed08 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -320,8 +320,9 @@ static void sun8i_mixer_mode_set(struct sunxi_engine 
*engine,
else
val = 0;
 
-   regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
-  SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val);
+   val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY;
+
+   regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base), val);
 
DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
 interlaced ? "on" : "off");
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index ebfc276b2464..bc12c95af6f3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -65,6 +65,7 @@
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2))
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)  ((n) << 2)
 
+#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY   BIT(0)
 #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1)
 
 #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)  BIT(ch)
-- 
2.30.2



Re: [PATCH] drm/sun4i: sun8i: Add the ability to keep scaler enabled for VI layer

2022-06-03 Thread Roman Stratiienko
Hi Maxime,

пт, 3 июн. 2022 г. в 11:24, Maxime Ripard :
>
> Hi,
>
> On Thu, Jun 02, 2022 at 06:01:18PM +, Roman Stratiienko wrote:
> > According to DE2.0/DE3.0 manual VI scaler enable register is double
> > buffered, but de facto it doesn't, or the hardware has the shadow
> > register latching issues which causes single-frame picture corruption
> > after changing the state of scaler enable register.
> >
> > Allow the user to keep the scaler always enabled, preventing the UI
> > glitches on the transition from scaled to unscaled state.
> >
> > NOTE:
> > UI layer scaler has more registers with double-buffering issue and can't
> > be workarounded in the same manner.
> >
> > You may find a python test and a demo video for this issue at [1]
> >
> > [1]: https://github.com/GloDroid/glodroid_tests/issues/4
>
> Please describe the issue entirely here. The commit log must be 
> self-sufficient.

Commit message already states "single-frame picture corruption after
changing the state of scaler enable register", therefore I find it
already self-sufficient

Also I find demo videos and link to tests useful for the followers to
go further with investigation.

If you have something specific in mind when asking to enhance the
commit message please say it.

>
> > Signed-off-by: Roman Stratiienko 
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 12 
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c |  4 +++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > index 71ab0a00b4de..15cad0330f66 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -27,6 +27,18 @@
> >  #include "sun8i_vi_layer.h"
> >  #include "sunxi_engine.h"
> >
> > +/* According to DE2.0/DE3.0 manual VI scaler enable register is double
> > + * buffered, but de facto it doesn't, or the hardware has the shadow
> > + * register latching issues which causes single-frame picture corruption
> > + * after changing the state of scaler enable register.
> > + * Allow the user to keep the scaler always enabled, preventing the UI
> > + * glitches on the transition from scaled to unscaled state.
> > + */
> > +int sun8i_vi_keep_scaler_enabled;
> > +MODULE_PARM_DESC(keep_vi_scaler_enabled,
> > +  "Keep VI scaler enabled (1 = enabled, 0 = disabled 
> > (default))");
> > +module_param_named(keep_vi_scaler_enabled, sun8i_vi_keep_scaler_enabled, 
> > int, 0644);
> > +
>
> It's not clear to me why we would want to make that a parameter?
>
>1 If it never works, we should fix it once and for all and not allow a broken 
>setup at all.

It's a hardware issue and can be fixed only within the hardware.

Current patch is a workaround that if enabled can cause increased
power consumption
for existing users. Therefore I think it is better to give existing
distro-maintainers a chance to test it prior to delivery.

Also I do not have all the sunxi SOCs to test it. My tests were
limited only by A64 and H6 SOCs.

> Maxime

Best regards,
Roman


Re: [PATCH] drm/sun4i: sun8i: Add the ability to keep scaler enabled for VI layer

2022-06-03 Thread Roman Stratiienko
Hi Ondrej,

пт, 3 июн. 2022 г. в 00:55, Ondřej Jirman :
>
> Hi Roman,
>
> On Thu, Jun 02, 2022 at 06:01:18PM +, Roman Stratiienko wrote:
> > According to DE2.0/DE3.0 manual VI scaler enable register is double
> > buffered, but de facto it doesn't, or the hardware has the shadow
> > register latching issues which causes single-frame picture corruption
> > after changing the state of scaler enable register.
> >
> > Allow the user to keep the scaler always enabled, preventing the UI
> > glitches on the transition from scaled to unscaled state.
> >
> > NOTE:
> > UI layer scaler has more registers with double-buffering issue and can't
> > be workarounded in the same manner.
> >
> > You may find a python test and a demo video for this issue at [1]
>
> Isn't this an issue with kernel driver not waiting for DE2 FINISH IRQ, but
> for VBLANK IRQ from TCON instead, before allowing to write new set of register
> values?

No, DE2 FINISH IRQ is triggered just some micro or even nanoseconds
earlier from vblank IRQ
(I have checked it using tracing).

I can guess MISSION FINISH IRQ triggered when the last line is being
sent to the line buffer cache (in between of the mixer output and tcon
input).
But VBLANG IRQ triggers when the last line is being sent to the
display + hfront porch time + hsync time.

I have also tried scheduling the register updates that have
double-buffering issues at VBLANK irq. And such a solution fixes the
test for both VI and UI scalers.
But under high loads there are still glitches happening. This patch
solves the issue for both test and high load conditions. (but for VI
only)
I'll post my solution with a deferred scaling register update soon.

>
> https://megous.com/dl/tmp/4fe35b3fc72ee7de.png
>
> I haven't checked if FINISH flag is set at time of VBLANK interrupt, so maybe
> this is not the issue.
>
> regards,
> o.
>
> > [1]: https://github.com/GloDroid/glodroid_tests/issues/4
> > Signed-off-by: Roman Stratiienko 
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 12 
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c |  4 +++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > index 71ab0a00b4de..15cad0330f66 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -27,6 +27,18 @@
> >  #include "sun8i_vi_layer.h"
> >  #include "sunxi_engine.h"
> >
> > +/* According to DE2.0/DE3.0 manual VI scaler enable register is double
> > + * buffered, but de facto it doesn't, or the hardware has the shadow
> > + * register latching issues which causes single-frame picture corruption
> > + * after changing the state of scaler enable register.
> > + * Allow the user to keep the scaler always enabled, preventing the UI
> > + * glitches on the transition from scaled to unscaled state.
> > + */
> > +int sun8i_vi_keep_scaler_enabled;
> > +MODULE_PARM_DESC(keep_vi_scaler_enabled,
> > +  "Keep VI scaler enabled (1 = enabled, 0 = disabled 
> > (default))");
> > +module_param_named(keep_vi_scaler_enabled, sun8i_vi_keep_scaler_enabled, 
> > int, 0644);
> > +
> >  struct de2_fmt_info {
> >   u32 drm_fmt;
> >   u32 de2_fmt;
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
> > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > index 662ba1018cc4..f005ab883503 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > @@ -17,6 +17,8 @@
> >  #include "sun8i_vi_layer.h"
> >  #include "sun8i_vi_scaler.h"
> >
> > +extern int sun8i_vi_keep_scaler_enabled;
> > +
> >  static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
> > int overlay, bool enable, unsigned int zpos)
> >  {
> > @@ -149,7 +151,7 @@ static int sun8i_vi_layer_update_coord(struct 
> > sun8i_mixer *mixer, int channel,
> >*/
> >   subsampled = format->hsub > 1 || format->vsub > 1;
> >
> > - if (insize != outsize || subsampled || hphase || vphase) {
> > + if (insize != outsize || subsampled || hphase || vphase || 
> > sun8i_vi_keep_scaler_enabled) {
> >   unsigned int scanline, required;
> >   struct drm_display_mode *mode;
> >   u32 hscale, vscale, fps;
> > --
> > 2.30.2
> >


[PATCH] drm/sun4i: sun8i: Add the ability to keep scaler enabled for VI layer

2022-06-02 Thread Roman Stratiienko
According to DE2.0/DE3.0 manual VI scaler enable register is double
buffered, but de facto it doesn't, or the hardware has the shadow
register latching issues which causes single-frame picture corruption
after changing the state of scaler enable register.

Allow the user to keep the scaler always enabled, preventing the UI
glitches on the transition from scaled to unscaled state.

NOTE:
UI layer scaler has more registers with double-buffering issue and can't
be workarounded in the same manner.

You may find a python test and a demo video for this issue at [1]

[1]: https://github.com/GloDroid/glodroid_tests/issues/4
Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 12 
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c |  4 +++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 71ab0a00b4de..15cad0330f66 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -27,6 +27,18 @@
 #include "sun8i_vi_layer.h"
 #include "sunxi_engine.h"
 
+/* According to DE2.0/DE3.0 manual VI scaler enable register is double
+ * buffered, but de facto it doesn't, or the hardware has the shadow
+ * register latching issues which causes single-frame picture corruption
+ * after changing the state of scaler enable register.
+ * Allow the user to keep the scaler always enabled, preventing the UI
+ * glitches on the transition from scaled to unscaled state.
+ */
+int sun8i_vi_keep_scaler_enabled;
+MODULE_PARM_DESC(keep_vi_scaler_enabled,
+"Keep VI scaler enabled (1 = enabled, 0 = disabled 
(default))");
+module_param_named(keep_vi_scaler_enabled, sun8i_vi_keep_scaler_enabled, int, 
0644);
+
 struct de2_fmt_info {
u32 drm_fmt;
u32 de2_fmt;
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 662ba1018cc4..f005ab883503 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -17,6 +17,8 @@
 #include "sun8i_vi_layer.h"
 #include "sun8i_vi_scaler.h"
 
+extern int sun8i_vi_keep_scaler_enabled;
+
 static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
  int overlay, bool enable, unsigned int zpos)
 {
@@ -149,7 +151,7 @@ static int sun8i_vi_layer_update_coord(struct sun8i_mixer 
*mixer, int channel,
 */
subsampled = format->hsub > 1 || format->vsub > 1;
 
-   if (insize != outsize || subsampled || hphase || vphase) {
+   if (insize != outsize || subsampled || hphase || vphase || 
sun8i_vi_keep_scaler_enabled) {
unsigned int scanline, required;
struct drm_display_mode *mode;
u32 hscale, vscale, fps;
-- 
2.30.2



[PATCH] drm/sun4i: Enable output signal premultiplication for DE2/DE3

2022-06-02 Thread Roman Stratiienko
Otherwise alpha value is discarded, resulting incorrect pixel
apperance on the display.

This also fixes missing transparency for the most bottom layer.

Test applications and videos w/ w/o this patch are available at [1].

[1]: https://github.com/GloDroid/glodroid_tests/issues/1
Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 ++
 drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 6b1711a9a71f..71ab0a00b4de 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -320,6 +320,8 @@ static void sun8i_mixer_mode_set(struct sunxi_engine 
*engine,
else
val = 0;
 
+   val |= SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY;
+
regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
   SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val);
 
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index ebfc276b2464..bc12c95af6f3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -65,6 +65,7 @@
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2))
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)  ((n) << 2)
 
+#define SUN8I_MIXER_BLEND_OUTCTL_PREMULTIPLY   BIT(0)
 #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1)
 
 #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)  BIT(ch)
-- 
2.30.2



Re: [PATCH] drm/sun4i: Fix blend route/enable register corruption for DE2.0/DE3.0

2022-05-30 Thread Roman Stratiienko
Hi Maxime,

пн, 30 мая 2022 г. в 16:22, Maxime Ripard :
>
> Hi Roman,
>
> On Wed, May 25, 2022 at 11:54:45AM +, Roman Stratiienko wrote:
> > By this commit 2 related issues are solved:
> >
> >   Issue #1. Corruption in blend route/enable register:
> >
> > Register corruption happens after using old_state->zpos to disable layer
> > state. Blend route/enable registers are shared with other layers
> > and other layers may have already assigned this PIPE to valid value.
> >
> > Solution: Do not use old_state->zpos to disable the plane pipe in
> > blend registers.
> >
> >   Issue #2. Remove disabled layer from blend route/enable registers:
> >
> > Since sun4i/drm are using normalized_zpos, .atomic_update() will setup
> > blend route/enable pipes starting from PIPE0 to PIPEX, where X+1 is a
> > number of layers used by the CRTC in this frame.
> >
> > Remaining pipes (PIPE[X+1] - PIPE[MAX]) can have old data that MUST be
> > updated.
> >
> > new_state->normalized_zpos can't be used, since drm helpers won't update
> > it for disabled planes.
> >
> > Solution:
> >
> > 1. Track the number of total used planes for crtc.
> > 2. Use this number instead of zpos to disable unused blend pipes.
> >
> > Signed-off-by: Roman Stratiienko 
>
> If there's two issues, and two solutions, it should be two patches.

I would say.. It's a single complex issue.
Solving one part without solving another will make things only worse.

>
> Maxime


Re: Re: Re: Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-25 Thread Roman Stratiienko
ср, 25 мая 2022 г. в 18:14, Jernej Škrabec :
>
> Dne sreda, 25. maj 2022 ob 16:55:56 CEST je Roman Stratiienko napisal(a):
> > вт, 24 мая 2022 г. в 22:14, Jernej Škrabec :
> > >
> > > Dne torek, 24. maj 2022 ob 19:14:35 CEST je Roman Stratiienko napisal(a):
> > > > By the way, not related to this issue:
> > > >
> > > > I cherry-picked
> > > > https://patchwork.kernel.org/project/dri-devel/patch/
> 20220424162633.12369-9-sam...@sholland.org/
> > > > and got a blank FB console on OPI3.
> > > > Can you check it please?
> > >
> > > Reply to that patch and we'll talk.
> >
> > Despite the fact that I am the original author of the patches I'm not
> > even in CC, so I can respond to this thread.
>
> Actually, many people come up with similar idea.
>
> > Is there any other way to respond to the message where you're not in CC?
>
> Of course, I download patch as mbox and import it in my e-mail client. A reply
> to it will continue the thread.

Nevermind, it was an issue from my side. I haven't done rmmod/insmod
for sun4i_tcon.ko after recompiling.

> Best regards,
> Jernej
>
> >
> > >
> > > Best regards,
> > > Jernej
> > >
> > > >
> > > > Regards,
> > > > Roman
> > > >
> > > >
> > > >
> > > > вт, 24 мая 2022 г. в 20:10, Roman Stratiienko :
> > > > >
> > > > > Please draft a test for the zpos issue you're mentioning.
> > > > >
> > > > > It's very easy to do with kmsxx using python wrapper.
> > > > >
> > > > > Or explain steps to reproduce here, I will write it by myself.
> > > > >
> > > > > Thanks.
> > > > > Regards
> > > > > Roman
> > > > >
> > > > > вт, 24 мая 2022 г. в 19:21, Jernej Škrabec :
> > > > > >
> > > > > > Dne torek, 24. maj 2022 ob 17:31:13 CEST je Roman Stratiienko
> > > napisal(a):
> > > > > > > NAK for this. Further testing showed such an approach is not
> reliable
> > > > > > > due to .atomic_update() callback called only in case planes have
> some
> > > > > > > changes.
> > > > > >
> > > > > > Additionally, I think it would be better to fix underlaying zpos
> issue
> > > first
> > > > > > (attempted many times) and then worry about blending.
> > > > > >
> > > > > > Best regards,
> > > > > > Jernej
> > > > > >
> > > > > > >
> > > > > > > вт, 24 мая 2022 г. в 16:52, Roman Stratiienko
> > > :
> > > > > > > >
> > > > > > > > Corruption happens when plane zpos is updated
> > > > > > > >
> > > > > > > > Example scenario:
> > > > > > > >
> > > > > > > > Initial frame blender state:
> > > > > > > > PLANE_ZPOS = {0, 1, 2, 3}
> > > > > > > > BLD_ROUTE  = {0, 1, 2, 0}
> > > > > > > > BLD_EN = {1, 1, 1, 0}
> > > > > > > >
> > > > > > > > New frame commit (Only ZPOS has been changed):
> > > > > > > >
> > > > > > > > PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
> > > > > > > >
> > > > > > > > Expected results after plane state update:
> > > > > > > > Z0 Z1 Z2 Z3
> > > > > > > > BLD_ROUTE = {1, 2, 0, 0}
> > > > > > > > BLD_EN= {1, 1, 1, 0}
> > > > > > > >
> > > > > > > > What is currently happening:
> > > > > > > >
> > > > > > > > 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> > > > > > > > BLD_ROUTE = {1->0, 1, 2->0, 0}
> > > > > > > > BLD_EN= {1->0, 1, 1->1, 0}
> > > > > > > >
> > > > > > > > 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> > > > > > > > BLD_ROUTE = {0->1, 1->0, 0, 0}
> > > > > > > > BLD_EN= {0->1, 1->0, 1, 0}
> > > > > > > >
> > > > > > > > 3. sun8i_ui_layer_enable(enabled=true, zpos=1, o

Re: Re: Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-25 Thread Roman Stratiienko
вт, 24 мая 2022 г. в 22:14, Jernej Škrabec :
>
> Dne torek, 24. maj 2022 ob 19:14:35 CEST je Roman Stratiienko napisal(a):
> > By the way, not related to this issue:
> >
> > I cherry-picked
> > https://patchwork.kernel.org/project/dri-devel/patch/20220424162633.12369-9-sam...@sholland.org/
> > and got a blank FB console on OPI3.
> > Can you check it please?
>
> Reply to that patch and we'll talk.

Despite the fact that I am the original author of the patches I'm not
even in CC, so I can respond to this thread.
Is there any other way to respond to the message where you're not in CC?

>
> Best regards,
> Jernej
>
> >
> > Regards,
> > Roman
> >
> >
> >
> > вт, 24 мая 2022 г. в 20:10, Roman Stratiienko :
> > >
> > > Please draft a test for the zpos issue you're mentioning.
> > >
> > > It's very easy to do with kmsxx using python wrapper.
> > >
> > > Or explain steps to reproduce here, I will write it by myself.
> > >
> > > Thanks.
> > > Regards
> > > Roman
> > >
> > > вт, 24 мая 2022 г. в 19:21, Jernej Škrabec :
> > > >
> > > > Dne torek, 24. maj 2022 ob 17:31:13 CEST je Roman Stratiienko
> napisal(a):
> > > > > NAK for this. Further testing showed such an approach is not reliable
> > > > > due to .atomic_update() callback called only in case planes have some
> > > > > changes.
> > > >
> > > > Additionally, I think it would be better to fix underlaying zpos issue
> first
> > > > (attempted many times) and then worry about blending.
> > > >
> > > > Best regards,
> > > > Jernej
> > > >
> > > > >
> > > > > вт, 24 мая 2022 г. в 16:52, Roman Stratiienko
> :
> > > > > >
> > > > > > Corruption happens when plane zpos is updated
> > > > > >
> > > > > > Example scenario:
> > > > > >
> > > > > > Initial frame blender state:
> > > > > > PLANE_ZPOS = {0, 1, 2, 3}
> > > > > > BLD_ROUTE  = {0, 1, 2, 0}
> > > > > > BLD_EN = {1, 1, 1, 0}
> > > > > >
> > > > > > New frame commit (Only ZPOS has been changed):
> > > > > >
> > > > > > PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
> > > > > >
> > > > > > Expected results after plane state update:
> > > > > > Z0 Z1 Z2 Z3
> > > > > > BLD_ROUTE = {1, 2, 0, 0}
> > > > > > BLD_EN= {1, 1, 1, 0}
> > > > > >
> > > > > > What is currently happening:
> > > > > >
> > > > > > 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> > > > > > BLD_ROUTE = {1->0, 1, 2->0, 0}
> > > > > > BLD_EN= {1->0, 1, 1->1, 0}
> > > > > >
> > > > > > 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> > > > > > BLD_ROUTE = {0->1, 1->0, 0, 0}
> > > > > > BLD_EN= {0->1, 1->0, 1, 0}
> > > > > >
> > > > > > 3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
> > > > > > BLD_ROUTE = {1, 0->2, 0->0, 0}
> > > > > > BLD_EN= {1, 0->2, 1->0, 0}
> > > > > >
> > > > > > After updating of all the planes we are ending up with BLD_EN[2]=0,
> > > > > > which makes this channel disabled.
> > > > > >
> > > > > > To fix this issue, clear BLEND registers before updating the planes
> > > > > > and do not clear the old state while processing every plane.
> > > > > >
> > > > > > Signed-off-by: Roman Stratiienko
> 
> > > > > > ---
> > > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
> > > > > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 +++
> +--
> > > > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 +++
> +--
> > > > > >  3 files changed, 24 insertions(+), 48 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/
> sun4i/
> > > > sun8i_mixer.c
> > > > > > index f5e8aeaa3cdf..004377a000fc 100644
&

Re: Re: Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-25 Thread Roman Stratiienko
вт, 24 мая 2022 г. в 22:13, Jernej Škrabec :
>
> Don't top post, it's annoying and against rules.
>
> Dne torek, 24. maj 2022 ob 19:10:06 CEST je Roman Stratiienko napisal(a):
> > Please draft a test for the zpos issue you're mentioning.
> >
> > It's very easy to do with kmsxx using python wrapper.
> >
> > Or explain steps to reproduce here, I will write it by myself.
>
> I'm talking about the issue which you, Ondrej Jirman and me all tried to fix 
> it
> in the past one way or another:
>
> https://patchwork.kernel.org/project/dri-devel/patch/20190914220337.646719-1-meg...@megous.com/
> https://patchwork.kernel.org/project/dri-devel/patch/20210106204630.1800284-1-jernej.skra...@siol.net/

This particular patch and the one I've sent  today [1] aims to fix the
ZPOS issue you're talking about.
Second patch [1] fixes https://github.com/rsglobal/kmsxx/pull/3 test
and unlike this patch it does not cause additional regressions (as far
as I can test).

[1]: 
https://patchwork.kernel.org/project/dri-devel/patch/20220525115445.93500-1-roman.o.stratiie...@globallogic.com/

>
> Best regards,
> Jernej
>
> >
> > Thanks.
> > Regards
> > Roman
> >
> > вт, 24 мая 2022 г. в 19:21, Jernej Škrabec :
> > >
> > > Dne torek, 24. maj 2022 ob 17:31:13 CEST je Roman Stratiienko napisal(a):
> > > > NAK for this. Further testing showed such an approach is not reliable
> > > > due to .atomic_update() callback called only in case planes have some
> > > > changes.
> > >
> > > Additionally, I think it would be better to fix underlaying zpos issue 
> > > first
> > > (attempted many times) and then worry about blending.
> > >
> > > Best regards,
> > > Jernej
> > >
> > > >
> > > > вт, 24 мая 2022 г. в 16:52, Roman Stratiienko :
> > > > >
> > > > > Corruption happens when plane zpos is updated
> > > > >
> > > > > Example scenario:
> > > > >
> > > > > Initial frame blender state:
> > > > > PLANE_ZPOS = {0, 1, 2, 3}
> > > > > BLD_ROUTE  = {0, 1, 2, 0}
> > > > > BLD_EN = {1, 1, 1, 0}
> > > > >
> > > > > New frame commit (Only ZPOS has been changed):
> > > > >
> > > > > PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
> > > > >
> > > > > Expected results after plane state update:
> > > > > Z0 Z1 Z2 Z3
> > > > > BLD_ROUTE = {1, 2, 0, 0}
> > > > > BLD_EN= {1, 1, 1, 0}
> > > > >
> > > > > What is currently happening:
> > > > >
> > > > > 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> > > > > BLD_ROUTE = {1->0, 1, 2->0, 0}
> > > > > BLD_EN= {1->0, 1, 1->1, 0}
> > > > >
> > > > > 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> > > > > BLD_ROUTE = {0->1, 1->0, 0, 0}
> > > > > BLD_EN= {0->1, 1->0, 1, 0}
> > > > >
> > > > > 3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
> > > > > BLD_ROUTE = {1, 0->2, 0->0, 0}
> > > > > BLD_EN= {1, 0->2, 1->0, 0}
> > > > >
> > > > > After updating of all the planes we are ending up with BLD_EN[2]=0,
> > > > > which makes this channel disabled.
> > > > >
> > > > > To fix this issue, clear BLEND registers before updating the planes
> > > > > and do not clear the old state while processing every plane.
> > > > >
> > > > > Signed-off-by: Roman Stratiienko 
> > > > > ---
> > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
> > > > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 +++
> +--
> > > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 +++
> +--
> > > > >  3 files changed, 24 insertions(+), 48 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/
> sun4i/
> > > sun8i_mixer.c
> > > > > index f5e8aeaa3cdf..004377a000fc 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > > @@ -248,6 +248,21 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32
> > > *hw_f

[PATCH] drm/sun4i: Fix blend route/enable register corruption for DE2.0/DE3.0

2022-05-25 Thread Roman Stratiienko
By this commit 2 related issues are solved:

  Issue #1. Corruption in blend route/enable register:

Register corruption happens after using old_state->zpos to disable layer
state. Blend route/enable registers are shared with other layers
and other layers may have already assigned this PIPE to valid value.

Solution: Do not use old_state->zpos to disable the plane pipe in
blend registers.

  Issue #2. Remove disabled layer from blend route/enable registers:

Since sun4i/drm are using normalized_zpos, .atomic_update() will setup
blend route/enable pipes starting from PIPE0 to PIPEX, where X+1 is a
number of layers used by the CRTC in this frame.

Remaining pipes (PIPE[X+1] - PIPE[MAX]) can have old data that MUST be
updated.

new_state->normalized_zpos can't be used, since drm helpers won't update
it for disabled planes.

Solution:

1. Track the number of total used planes for crtc.
2. Use this number instead of zpos to disable unused blend pipes.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 +
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 60 +-
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  2 +
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 58 +
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  2 +
 5 files changed, 47 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 5b3fbee18671..ebfc276b2464 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -177,6 +177,8 @@ struct sun8i_mixer {
 
struct clk  *bus_clk;
struct clk  *mod_clk;
+
+   int used_layers;
 };
 
 static inline struct sun8i_mixer *
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 7845c2a53a7f..ca79cb4d5c04 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -24,8 +24,7 @@
 #include "sun8i_ui_scaler.h"
 
 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;
 
@@ -44,32 +43,18 @@ 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);
 
-   if (!enable || zpos != 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 = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
+   val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
-  val, val);
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
+  val, enable ? val : 0);
 
-   val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos);
+   val = 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);
-   }
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_ROUTE(bld_base),
+  SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos),
+  val);
 }
 
 static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
@@ -291,32 +276,29 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane 
*plane,
 static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane,
  struct drm_atomic_state *state)
 {
-   struct drm_plane_state *old_state = 
drm_atomic_get_old_plane_state(state,
-  
plane);
struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
-   unsigned int old_zpos = old_state->normalized_zpos;
struct sun8i_mixer *mixer = layer->mixer;
 
-   sun8i_ui_la

Re: Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-24 Thread Roman Stratiienko
By the way, not related to this issue:

I cherry-picked
https://patchwork.kernel.org/project/dri-devel/patch/20220424162633.12369-9-sam...@sholland.org/
and got a blank FB console on OPI3.
Can you check it please?

Regards,
Roman



вт, 24 мая 2022 г. в 20:10, Roman Stratiienko :
>
> Please draft a test for the zpos issue you're mentioning.
>
> It's very easy to do with kmsxx using python wrapper.
>
> Or explain steps to reproduce here, I will write it by myself.
>
> Thanks.
> Regards
> Roman
>
> вт, 24 мая 2022 г. в 19:21, Jernej Škrabec :
> >
> > Dne torek, 24. maj 2022 ob 17:31:13 CEST je Roman Stratiienko napisal(a):
> > > NAK for this. Further testing showed such an approach is not reliable
> > > due to .atomic_update() callback called only in case planes have some
> > > changes.
> >
> > Additionally, I think it would be better to fix underlaying zpos issue first
> > (attempted many times) and then worry about blending.
> >
> > Best regards,
> > Jernej
> >
> > >
> > > вт, 24 мая 2022 г. в 16:52, Roman Stratiienko :
> > > >
> > > > Corruption happens when plane zpos is updated
> > > >
> > > > Example scenario:
> > > >
> > > > Initial frame blender state:
> > > > PLANE_ZPOS = {0, 1, 2, 3}
> > > > BLD_ROUTE  = {0, 1, 2, 0}
> > > > BLD_EN = {1, 1, 1, 0}
> > > >
> > > > New frame commit (Only ZPOS has been changed):
> > > >
> > > > PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
> > > >
> > > > Expected results after plane state update:
> > > > Z0 Z1 Z2 Z3
> > > > BLD_ROUTE = {1, 2, 0, 0}
> > > > BLD_EN= {1, 1, 1, 0}
> > > >
> > > > What is currently happening:
> > > >
> > > > 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> > > > BLD_ROUTE = {1->0, 1, 2->0, 0}
> > > > BLD_EN= {1->0, 1, 1->1, 0}
> > > >
> > > > 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> > > > BLD_ROUTE = {0->1, 1->0, 0, 0}
> > > > BLD_EN= {0->1, 1->0, 1, 0}
> > > >
> > > > 3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
> > > > BLD_ROUTE = {1, 0->2, 0->0, 0}
> > > > BLD_EN= {1, 0->2, 1->0, 0}
> > > >
> > > > After updating of all the planes we are ending up with BLD_EN[2]=0,
> > > > which makes this channel disabled.
> > > >
> > > > To fix this issue, clear BLEND registers before updating the planes
> > > > and do not clear the old state while processing every plane.
> > > >
> > > > Signed-off-by: Roman Stratiienko 
> > > > ---
> > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
> > > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 --
> > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 --
> > > >  3 files changed, 24 insertions(+), 48 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> > > > b/drivers/gpu/drm/sun4i/
> > sun8i_mixer.c
> > > > index f5e8aeaa3cdf..004377a000fc 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > @@ -248,6 +248,21 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32
> > *hw_format)
> > > > return -EINVAL;
> > > >  }
> > > >
> > > > +static void sun8i_atomic_begin(struct sunxi_engine *engine,
> > > > +  struct drm_crtc_state *old_state)
> > > > +{
> > > > +   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > > > +   u32 bld_base = sun8i_blender_base(mixer);
> > > > +
> > > > +   regmap_write(engine->regs,
> > > > +SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
> > > > +0);
> > > > +
> > > > +   regmap_write(engine->regs,
> > > > +SUN8I_MIXER_BLEND_ROUTE(bld_base),
> > > > +0);
> > > > +}
> > > > +
> > > >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> > > >  {
> > > > DRM_DEBUG_DRIVER("Committing changes\n");

Re: Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-24 Thread Roman Stratiienko
Please draft a test for the zpos issue you're mentioning.

It's very easy to do with kmsxx using python wrapper.

Or explain steps to reproduce here, I will write it by myself.

Thanks.
Regards
Roman

вт, 24 мая 2022 г. в 19:21, Jernej Škrabec :
>
> Dne torek, 24. maj 2022 ob 17:31:13 CEST je Roman Stratiienko napisal(a):
> > NAK for this. Further testing showed such an approach is not reliable
> > due to .atomic_update() callback called only in case planes have some
> > changes.
>
> Additionally, I think it would be better to fix underlaying zpos issue first
> (attempted many times) and then worry about blending.
>
> Best regards,
> Jernej
>
> >
> > вт, 24 мая 2022 г. в 16:52, Roman Stratiienko :
> > >
> > > Corruption happens when plane zpos is updated
> > >
> > > Example scenario:
> > >
> > > Initial frame blender state:
> > > PLANE_ZPOS = {0, 1, 2, 3}
> > > BLD_ROUTE  = {0, 1, 2, 0}
> > > BLD_EN = {1, 1, 1, 0}
> > >
> > > New frame commit (Only ZPOS has been changed):
> > >
> > > PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
> > >
> > > Expected results after plane state update:
> > > Z0 Z1 Z2 Z3
> > > BLD_ROUTE = {1, 2, 0, 0}
> > > BLD_EN= {1, 1, 1, 0}
> > >
> > > What is currently happening:
> > >
> > > 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> > > BLD_ROUTE = {1->0, 1, 2->0, 0}
> > > BLD_EN= {1->0, 1, 1->1, 0}
> > >
> > > 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> > > BLD_ROUTE = {0->1, 1->0, 0, 0}
> > > BLD_EN= {0->1, 1->0, 1, 0}
> > >
> > > 3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
> > >     BLD_ROUTE = {1, 0->2, 0->0, 0}
> > > BLD_EN= {1, 0->2, 1->0, 0}
> > >
> > > After updating of all the planes we are ending up with BLD_EN[2]=0,
> > > which makes this channel disabled.
> > >
> > > To fix this issue, clear BLEND registers before updating the planes
> > > and do not clear the old state while processing every plane.
> > >
> > > Signed-off-by: Roman Stratiienko 
> > > ---
> > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
> > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 --
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 --
> > >  3 files changed, 24 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/
> sun8i_mixer.c
> > > index f5e8aeaa3cdf..004377a000fc 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > @@ -248,6 +248,21 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32
> *hw_format)
> > > return -EINVAL;
> > >  }
> > >
> > > +static void sun8i_atomic_begin(struct sunxi_engine *engine,
> > > +  struct drm_crtc_state *old_state)
> > > +{
> > > +   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > > +   u32 bld_base = sun8i_blender_base(mixer);
> > > +
> > > +   regmap_write(engine->regs,
> > > +SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
> > > +0);
> > > +
> > > +   regmap_write(engine->regs,
> > > +SUN8I_MIXER_BLEND_ROUTE(bld_base),
> > > +0);
> > > +}
> > > +
> > >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> > >  {
> > > DRM_DEBUG_DRIVER("Committing changes\n");
> > > @@ -299,6 +314,7 @@ static struct drm_plane **sun8i_layers_init(struct
> drm_device *drm,
> > >  }
> > >
> > >  static const struct sunxi_engine_ops sun8i_engine_ops = {
> > > +   .atomic_begin   = sun8i_atomic_begin,
> > > .commit = sun8i_mixer_commit,
> > > .layers_init= sun8i_layers_init,
> > >  };
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/
> sun4i/sun8i_ui_layer.c
> > > index 7845c2a53a7f..b294a882626a 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > @@ -24,8 +24,7 @@
> > >  #include "sun8i_ui_scaler.h"
> > >

Re: [PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-24 Thread Roman Stratiienko
NAK for this. Further testing showed such an approach is not reliable
due to .atomic_update() callback called only in case planes have some
changes.

вт, 24 мая 2022 г. в 16:52, Roman Stratiienko :
>
> Corruption happens when plane zpos is updated
>
> Example scenario:
>
> Initial frame blender state:
> PLANE_ZPOS = {0, 1, 2, 3}
> BLD_ROUTE  = {0, 1, 2, 0}
> BLD_EN = {1, 1, 1, 0}
>
> New frame commit (Only ZPOS has been changed):
>
> PLANE_ZPOS = {0->2, 1->0, 2->1, 3}
>
> Expected results after plane state update:
> Z0 Z1 Z2 Z3
> BLD_ROUTE = {1, 2, 0, 0}
> BLD_EN= {1, 1, 1, 0}
>
> What is currently happening:
>
> 1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
> BLD_ROUTE = {1->0, 1, 2->0, 0}
> BLD_EN= {1->0, 1, 1->1, 0}
>
> 2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
> BLD_ROUTE = {0->1, 1->0, 0, 0}
> BLD_EN= {0->1, 1->0, 1, 0}
>
> 3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
> BLD_ROUTE = {1, 0->2, 0->0, 0}
> BLD_EN= {1, 0->2, 1->0, 0}
>
> After updating of all the planes we are ending up with BLD_EN[2]=0,
> which makes this channel disabled.
>
> To fix this issue, clear BLEND registers before updating the planes
> and do not clear the old state while processing every plane.
>
> Signed-off-by: Roman Stratiienko 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 --
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 --
>  3 files changed, 24 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index f5e8aeaa3cdf..004377a000fc 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -248,6 +248,21 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 
> *hw_format)
> return -EINVAL;
>  }
>
> +static void sun8i_atomic_begin(struct sunxi_engine *engine,
> +  struct drm_crtc_state *old_state)
> +{
> +   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +   u32 bld_base = sun8i_blender_base(mixer);
> +
> +   regmap_write(engine->regs,
> +SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
> +0);
> +
> +   regmap_write(engine->regs,
> +SUN8I_MIXER_BLEND_ROUTE(bld_base),
> +0);
> +}
> +
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
> DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -299,6 +314,7 @@ static struct drm_plane **sun8i_layers_init(struct 
> drm_device *drm,
>  }
>
>  static const struct sunxi_engine_ops sun8i_engine_ops = {
> +   .atomic_begin   = sun8i_atomic_begin,
> .commit = sun8i_mixer_commit,
> .layers_init= sun8i_layers_init,
>  };
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 7845c2a53a7f..b294a882626a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -24,8 +24,7 @@
>  #include "sun8i_ui_scaler.h"
>
>  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;
>
> @@ -44,18 +43,6 @@ 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);
>
> -   if (!enable || zpos != 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 = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
>
> @@ -291,31 +278,24 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane 
> *plane,
> 

[PATCH] drm/sun4i: Fix blend registers corruption for DE2.0/DE3.0

2022-05-24 Thread Roman Stratiienko
Corruption happens when plane zpos is updated

Example scenario:

Initial frame blender state:
PLANE_ZPOS = {0, 1, 2, 3}
BLD_ROUTE  = {0, 1, 2, 0}
BLD_EN = {1, 1, 1, 0}

New frame commit (Only ZPOS has been changed):

PLANE_ZPOS = {0->2, 1->0, 2->1, 3}

Expected results after plane state update:
Z0 Z1 Z2 Z3
BLD_ROUTE = {1, 2, 0, 0}
BLD_EN= {1, 1, 1, 0}

What is currently happening:

1. sun8i_vi_layer_enable(enabled=true, zpos=2, old_zpos=0):
BLD_ROUTE = {1->0, 1, 2->0, 0}
BLD_EN= {1->0, 1, 1->1, 0}

2. sun8i_ui_layer_enable(enabled=true, zpos=0, old_zpos=1):
BLD_ROUTE = {0->1, 1->0, 0, 0}
BLD_EN= {0->1, 1->0, 1, 0}

3. sun8i_ui_layer_enable(enabled=true, zpos=1, old_zpos=2):
BLD_ROUTE = {1, 0->2, 0->0, 0}
BLD_EN= {1, 0->2, 1->0, 0}

After updating of all the planes we are ending up with BLD_EN[2]=0,
which makes this channel disabled.

To fix this issue, clear BLEND registers before updating the planes
and do not clear the old state while processing every plane.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 16 +++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 28 --
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 --
 3 files changed, 24 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index f5e8aeaa3cdf..004377a000fc 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -248,6 +248,21 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 
*hw_format)
return -EINVAL;
 }
 
+static void sun8i_atomic_begin(struct sunxi_engine *engine,
+  struct drm_crtc_state *old_state)
+{
+   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+   u32 bld_base = sun8i_blender_base(mixer);
+
+   regmap_write(engine->regs,
+SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
+0);
+
+   regmap_write(engine->regs,
+SUN8I_MIXER_BLEND_ROUTE(bld_base),
+0);
+}
+
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
DRM_DEBUG_DRIVER("Committing changes\n");
@@ -299,6 +314,7 @@ static struct drm_plane **sun8i_layers_init(struct 
drm_device *drm,
 }
 
 static const struct sunxi_engine_ops sun8i_engine_ops = {
+   .atomic_begin   = sun8i_atomic_begin,
.commit = sun8i_mixer_commit,
.layers_init= sun8i_layers_init,
 };
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 7845c2a53a7f..b294a882626a 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -24,8 +24,7 @@
 #include "sun8i_ui_scaler.h"
 
 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;
 
@@ -44,18 +43,6 @@ 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);
 
-   if (!enable || zpos != 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 = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
@@ -291,31 +278,24 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane 
*plane,
 static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane,
  struct drm_atomic_state *state)
 {
-   struct drm_plane_state *old_state = 
drm_atomic_get_old_plane_state(state,
-  
plane);
struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
-   unsigned int old_zpos = old_state->normalized_zpos;
struct sun8i_mixer *mixer = layer->mixer;
 
-   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
- old_zpos);
+   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0);
 }
 
 static void sun8i_ui_layer_atomic_update(struct drm_plane *pl

Re: Re: DE2.0 YV12 playback issues after ea067aee45a8

2021-08-31 Thread Roman Stratiienko
Hi Jernej,

вт, 31 авг. 2021 г. в 19:52, Jernej Škrabec :
>
> Hi!
>
> Dne petek, 27. avgust 2021 ob 15:16:03 CEST je Roman Stratiienko napisal(a):
> > +CC: jernej.skra...@gmail.com
> >
> > пт, 27 авг. 2021 г. в 16:12, Roman Stratiienko :
> > >
> > > Hello Jernej,
> > >
> > > During local testing I faced an issue where YV12 buffers are displayed
> > > all in blue.
> > >
> > > Issue can be fixed by reverting:
> > > ea067aee45a8 ("drm/sun4i: de2/de3: Remove redundant CSC matrices")
> > >
> > > Could you have a look please?
>
> I believe I found the issue. Can you try replacing this line:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/sun4i/
> sun8i_csc.h#L20
>
> with:
> #define SUN8I_CSC_COEFF(base, i)(base + 0x10 + 4 * (i))
>
> Note that "i" is in parenthesis.

Thank you for your quick response.
Yeah, that's it. Adding parenthesis solves the issue.

>
> Best regards,
> Jernej
>
> > >
> > > Best regards,
> > > Roman Stratiienko
> >
>
>


DE2.0 YV12 playback issues after ea067aee45a8

2021-08-27 Thread Roman Stratiienko
Hello Jernej,

During local testing I faced an issue where YV12 buffers are displayed
all in blue.

Issue can be fixed by reverting:
ea067aee45a8 ("drm/sun4i: de2/de3: Remove redundant CSC matrices")

Could you have a look please?

Best regards,
Roman Stratiienko


Re: DE2.0 YV12 playback issues after ea067aee45a8

2021-08-27 Thread Roman Stratiienko
+CC: jernej.skra...@gmail.com

пт, 27 авг. 2021 г. в 16:12, Roman Stratiienko :
>
> Hello Jernej,
>
> During local testing I faced an issue where YV12 buffers are displayed
> all in blue.
>
> Issue can be fixed by reverting:
> ea067aee45a8 ("drm/sun4i: de2/de3: Remove redundant CSC matrices")
>
> Could you have a look please?
>
> Best regards,
> Roman Stratiienko


[PATCH v4 1/2] drm/sun4i: Add mode_set callback to the engine

2021-05-29 Thread Roman Stratiienko
Create callback to allow updating engine's registers on mode change.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
 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 45d9eb552d86..8f01a6b2bbef 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -146,6 +146,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.30.2



Re: [PATCH v4 2/2] drm/sun4i: Use CRTC size instead of primary plane size as mixer frame

2021-05-29 Thread Roman Stratiienko
+CC: jernej.skra...@gmail.com

пт, 28 мая 2021 г. в 23:31, Roman Stratiienko :
>
> Fixes corrupted display picture when primary plane isn't full-screen.
>
> Signed-off-by: Roman Stratiienko 
> ---
>  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);
> +   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);
> --
> 2.30.2
>


[PATCH v4 0/2] drm/sun4i: Set mixer frame to display size for DE2.0+

2021-05-29 Thread Roman Stratiienko
This patchset fixes corrupted display picture when primary plane isn't
full-screen.

Please review/merge.

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

v4:
Resend (author's email changed). Reword commit message.
Drop fixes line to allow more testing before back-porting.

 drivers/gpu/drm/sun4i/sun4i_crtc.c |  3 +++
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 28 
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --
 drivers/gpu/drm/sun4i/sunxi_engine.h   | 12 
 4 files changed, 43 insertions(+), 30 deletions(-)



[PATCH v4 2/2] drm/sun4i: Use CRTC size instead of primary plane size as mixer frame

2021-05-29 Thread Roman Stratiienko
Fixes corrupted display picture when primary plane isn't full-screen.

Signed-off-by: Roman Stratiienko 
---
 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);
+   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);
-- 
2.30.2



[PATCH v5 0/2] Implement DE2.0 and DE3.0 per-plane alpha support

2021-01-28 Thread Roman Stratiienko


Please review/merge.

v2:
Initial patch

v3:
- Skip adding & applying alpha property if VI count > 1 (v3s case)

v4:
Resend (author's email changed)

v5:
Resend

drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 +
drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 48 

drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 +++
4 files changed, 85 insertions(+), 8 deletions(-)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 2/2] drm/sun4i: Add alpha property for sun8i and sun50i VI layer

2021-01-28 Thread Roman Stratiienko
DE3.0 VI layers supports plane-global alpha channel.
DE2.0 FCC block have GLOBAL_ALPHA register that can be used as alpha source
for blender.

Add alpha property to the DRM plane and connect it to the
corresponding registers in the mixer.

Do not add alpha property for V3s SOC that have DE2.0 and 2 VI planes.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 48 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 ++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 76393fc976fe..684462ce2a9b 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -66,6 +66,36 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   if (mixer->cfg->is_de3) {
+   mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
+  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+   val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
+   (plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
+ overlay),
+  mask, val);
+   } else if (mixer->cfg->vi_num == 1) {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA
+   (plane->state->alpha >> 8));
+   }
+}
+
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -268,14 +298,6 @@ static int sun8i_vi_layer_update_formats(struct 
sun8i_mixer *mixer, int channel,
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 
-   /* It seems that YUV formats use global alpha setting. */
-   if (mixer->cfg->is_de3)
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
- overlay),
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK,
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(0xff));
-
return 0;
 }
 
@@ -393,6 +415,8 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_vi_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_vi_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_vi_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -534,6 +558,14 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   if (mixer->cfg->vi_num == 1 || mixer->cfg->is_de3) {
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+   }
+
ret = drm_plane_create_zpos_property(>plane, index,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index eaa6076f5dbc..48c399e1c86d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -29,14 +29,25 @@
 #define SUN8I_MIXER_CHAN_VI_VDS_UV(base) \
((base) + 0xfc)
 
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG \
+   (0xAA000 + 0x90)
+
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA(x)((x) << 24)
+#define SUN8I_MI

[PATCH v5 1/2] drm/sun4i: Add alpha property for sun8i UI layer

2021-01-28 Thread Roman Stratiienko
DE2.0 and DE3.0 UI layers supports plane-global alpha channel.
Add alpha property to the DRM plane and connect it to the
corresponding registers in mixer.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 816ad4ce8996..e64f30595f64 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,6 +72,27 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+
+   val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+  mask, val);
+}
+
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -290,6 +311,8 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_ui_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_ui_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -367,6 +390,12 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+
ret = drm_plane_create_zpos_property(>plane, channel,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
index f4ab1cf6cded..e3e32ee1178d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -40,6 +40,11 @@
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK  GENMASK(12, 8)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET8
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK  GENMASK(31, 24)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(x)((x) << 24)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL((0) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER((1) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
 
 struct sun8i_mixer;
 
-- 
2.27.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/sun4i: Fix DE2 YVU handling

2020-09-03 Thread Roman Stratiienko
ср, 2 сент. 2020 г. в 19:46, Jernej Škrabec :
>
> Dne sreda, 02. september 2020 ob 09:01:17 CEST je Roman Stratiienko
> napisal(a):
> > ср, 2 сент. 2020 г. в 00:58, Jernej Skrabec :
> > > Function sun8i_vi_layer_get_csc_mode() is supposed to return CSC mode
> > > but due to inproper return type (bool instead of u32) it returns just 0
> > > or 1. Colors are wrong for YVU formats because of that.
> > >
> > > Fixes: daab3d0e8e2b ("drm/sun4i: de2: csc_mode in de2 format struct is
> > > mostly redundant") Reported-by: Roman Stratiienko
> > > 
> > > Signed-off-by: Jernej Skrabec 
> > > ---
> > >
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 22c8c5375d0d..c0147af6a840
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > @@ -211,7 +211,7 @@ static int sun8i_vi_layer_update_coord(struct
> > > sun8i_mixer *mixer, int channel,>
> > > return 0;
> > >
> > >  }
> > >
> > > -static bool sun8i_vi_layer_get_csc_mode(const struct drm_format_info
> > > *format) +static u32 sun8i_vi_layer_get_csc_mode(const struct
> > > drm_format_info *format)>
> > >  {
> > >
> > > if (!format->is_yuv)
> > >
> > > return SUN8I_CSC_MODE_OFF;
> > >
> > > --
> > > 2.28.0
> >
> > Hi Jernej,
> >
> > Thank you for the fix.
> > I can confirm this patch fixes the issue with wrong colors.
>
> Thanks! Can I assume that this means your Tested-by tag can be added?

Sure.

Tested-by: Roman Stratiienko 

>
> >
> > Let me share my thoughts:
> > I've looked into csc code, and it seems to me reordering U, V offsets
> > should be a much simpler solution than applying
> > color transformation matrices.It should also simplify adding more
> > color encodings in the future.
>
> Switching offsets assumes that you have separate planes for U and V which may
> not be true in the future. I agree that CSC matrices are needlessly duplicated
> for handling U/V switch. I have a patch which reorganize matrix on the fly 
> when
> coefficients are written in registers but since it's a part of a bigger,
> unfinished series, I didn't sent it out yet. Only difference in YUV and YVU 
> CSC
> matrices are switched 2nd and 3rd column.

This sounds reasonable. Thank you for your explanation.

Regards,
Roman

>
> Best regards,
> Jernej
>
> >
> > Regards,
> > Roman
>
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/sun4i: Fix DE2 YVU handling

2020-09-02 Thread Roman Stratiienko
ср, 2 сент. 2020 г. в 00:58, Jernej Skrabec :
>
> Function sun8i_vi_layer_get_csc_mode() is supposed to return CSC mode
> but due to inproper return type (bool instead of u32) it returns just 0
> or 1. Colors are wrong for YVU formats because of that.
>
> Fixes: daab3d0e8e2b ("drm/sun4i: de2: csc_mode in de2 format struct is mostly 
> redundant")
> Reported-by: Roman Stratiienko 
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> index 22c8c5375d0d..c0147af6a840 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -211,7 +211,7 @@ static int sun8i_vi_layer_update_coord(struct sun8i_mixer 
> *mixer, int channel,
> return 0;
>  }
>
> -static bool sun8i_vi_layer_get_csc_mode(const struct drm_format_info *format)
> +static u32 sun8i_vi_layer_get_csc_mode(const struct drm_format_info *format)
>  {
> if (!format->is_yuv)
> return SUN8I_CSC_MODE_OFF;
> --
> 2.28.0
>

Hi Jernej,

Thank you for the fix.
I can confirm this patch fixes the issue with wrong colors.

Let me share my thoughts:
I've looked into csc code, and it seems to me reordering U, V offsets
should be a much simpler solution than applying
color transformation matrices.It should also simplify adding more
color encodings in the future.

Regards,
Roman
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] RFC: sun4i/drm: Swap back U and V channels for DRM_FORMAT_YVU4xx

2020-09-02 Thread Roman Stratiienko
Fixes: e1ef9006663b ("drm/sun4i: Wire in DE2 YUV support")
Signed-off-by: Roman Stratiienko 

---
CC: meg...@megous.com
CC: jernej.skra...@gmail.com
CC: linux-su...@googlegroups.com
CC: dri-devel@lists.freedesktop.org
CC: linux-arm-ker...@lists.infradead.org
CC: linux-ker...@vger.kernel.org

Hi, this patch fixes wrong colors during video playback for me.
Implemented ugly for now, please review/suggest how to improve.
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c|  8 +++-
 drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 +-
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c |  2 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 28 +++---
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index dce40c430100..bbbeef44899a 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -31,6 +31,7 @@
 struct de2_fmt_info {
u32 drm_fmt;
u32 de2_fmt;
+   boolswap_uv;
 };
 
 static bool hw_preconfigured;
@@ -219,14 +220,17 @@ static const struct de2_fmt_info de2_formats[] = {
{
.drm_fmt = DRM_FORMAT_YVU422,
.de2_fmt = SUN8I_MIXER_FBFMT_YUV422,
+   .swap_uv = true,
},
{
.drm_fmt = DRM_FORMAT_YVU420,
.de2_fmt = SUN8I_MIXER_FBFMT_YUV420,
+   .swap_uv = true,
},
{
.drm_fmt = DRM_FORMAT_YVU411,
.de2_fmt = SUN8I_MIXER_FBFMT_YUV411,
+   .swap_uv = true,
},
{
.drm_fmt = DRM_FORMAT_P010,
@@ -238,13 +242,15 @@ static const struct de2_fmt_info de2_formats[] = {
},
 };
 
-int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format)
+int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format, bool *swap_uv)
 {
unsigned int i;
 
for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format) {
*hw_format = de2_formats[i].de2_fmt;
+   if (swap_uv)
+   *swap_uv = de2_formats[i].swap_uv;
return 0;
}
 
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 79a74bca1ea3..6358ffd251f9 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -207,5 +207,5 @@ sun8i_channel_base(struct sun8i_mixer *mixer, int channel)
return DE2_CH_BASE + channel * DE2_CH_SIZE;
 }
 
-int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format);
+int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format, bool *swap_uv);
 #endif /* _SUN8I_MIXER_H_ */
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index a7f21f08ec89..57bbd9f1071c 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -215,7 +215,7 @@ static int sun8i_ui_layer_update_formats(struct sun8i_mixer 
*mixer, int channel,
ch_base = sun8i_channel_base(mixer, channel);
 
fmt = state->fb->format;
-   ret = sun8i_mixer_drm_format_to_hw(fmt->format, _fmt);
+   ret = sun8i_mixer_drm_format_to_hw(fmt->format, _fmt, NULL);
if (ret || fmt->is_yuv) {
DRM_DEBUG_DRIVER("Invalid format\n");
return -EINVAL;
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 3553e38ec642..4da51155c4d5 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -313,7 +313,7 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer 
*mixer, int channel,
ch_base = sun8i_channel_base(mixer, channel);
 
fmt = state->fb->format;
-   ret = sun8i_mixer_drm_format_to_hw(fmt->format, _fmt);
+   ret = sun8i_mixer_drm_format_to_hw(fmt->format, _fmt, NULL);
if (ret) {
DRM_DEBUG_DRIVER("Invalid format\n");
return ret;
@@ -368,8 +368,17 @@ static int sun8i_vi_layer_update_buffer(struct sun8i_mixer 
*mixer, int channel,
struct drm_gem_cma_object *gem;
u32 dx, dy, src_x, src_y;
dma_addr_t paddr;
+   bool swap_uv;
u32 ch_base;
-   int i;
+   u32 hw_fmt;
+   int ret;
+   int i, j;
+
+   ret = sun8i_mixer_drm_format_to_hw(plane->state->fb->format->format, 
_fmt, _uv);
+   if (ret) {
+   DRM_DEBUG_DRIVER("Invalid format\n");
+   return ret;
+   }
 
ch_base = sun8i_channel_base(mixer, channel);
 
@@ -377,7 +386,12 @@ static int sun8i_vi_layer_update_buffer(struct sun8i_mixer 
*mixer, int channel,
src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
src_y = (state->src.y1 >> 16) & ~(format

[PATCH v4 3/4] drm/sun4i: Add support for premulti/coverage blending mode handling

2020-03-03 Thread Roman Stratiienko
Argument:

All information below is author's understanding of underlying processes.
This information was obtained from DE2.0-3.0 datasheet analysis and
analysis of different DRM driver source code and commit messages.

Input buffers could have 2 ways of holding pixel alpha channel value.
1. Coverage means that in case of transparency - only alpha channel changes.
   Example of 50% transparent white pixel: R=0xff B=0xff B=0xff A=0x7f

2. Premultiply means that RGB pixel values should be dimmed proportional to
   alpha channel value. Alpha channel value has also to be set in this case.
   Example of 50% transparent white pixel: R=0x7f B=0x7f B=0x7f A=0x7f

Userspace informs DRM/KMS which blending type frame buffer uses with
'pixel blend mode' property.

Allwinner DE2.0 and DE3.0 have 2 block of image processing:
Overlay and Blending. Both should aware of blending type are used in the buffer.

Overlay block uses this information to:
1. Unify blending types if more then 1 overlay channel are used. It can unify it
   only as premultiplied by converting coverage to premultiplied.
2. Calculate correct pixel value in case of applying plane alpha.
   For coverage alpha only alpha channel should be affected:
 [Ro=Ri, Go=Gi, Bo=Bi, Ao=Ai*AGlobal]
   For premultiplied alpha all 4 channels should be affected:
 [Ro=Ri*AGlobal, Go=Gi*AGlobal, Bo=Bi*AGlobal, Ao=Ai*AGlobal]

Blending functional block should aware of blending type each pipe
channel uses. Otherwise image can't blend correctly.

In case we've specified premultiplied format for blending PIPE0, blender
converts premultiplied RGB values to original (divides by normalized Alpha).
In case for some reason pixel value after division exceeds 0xff, blender clamps
it to 0xff. [Was discovered in experimental way]
If image that passed through PIPE1-3 restored to coverage before mixing or used
in premultiplied form still require testing and out of scope of this patch.

Implementation:

1. Add blend property to UI channel
2. Add blend property to VI channel in case of DE3.0 used
3. Make all DE2.0 UI and DE3.0 VI overlay channels to use premultiply format.
   Mark all blending pipes as premultiply except DE2.0 VI plane.
4. If DRM plane uses coverage blending format, set blending mode register
   to convert it to premultiply.

Signed-off-by: Roman Stratiienko 

--

v4:
- Initial version (Depends on unmerged patches from patchset)
---
 drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ++-
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  5 
 5 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 915479cc3077..8a18372938d5 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -70,6 +70,8 @@
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)(0xf << ((n) << 2))
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)  ((n) << 2)
 
+#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe) BIT(pipe)
+
 #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACEDBIT(1)
 
 #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)  BIT(ch)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 5278032567a3..dd6145f80c36 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -73,10 +73,12 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer 
*mixer, int channel,
 }
 
 static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
-   int overlay, struct drm_plane *plane)
+   int overlay, struct drm_plane *plane,
+   unsigned int zpos)
 {
-   u32 mask, val, ch_base;
+   u32 mask, val, ch_base, bld_base;
 
+   bld_base = sun8i_blender_base(mixer);
ch_base = sun8i_channel_base(mixer, channel);
 
mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
@@ -84,13 +86,27 @@ static void sun8i_ui_layer_update_alpha(struct sun8i_mixer 
*mixer, int channel,
 
val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
 
-   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
-   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
-   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+   if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+   val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
+   } else {
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+ 

[PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer

2020-03-03 Thread Roman Stratiienko
Allwinner display engine blender consists of 3 pipelined blending units.

PIPE0->\
BLD0-\
PIPE1->/  BLD1-\
PIPE2->--/  BLD2->OUT
PIPE3->/

This pipeline produces incorrect composition if PIPE0 buffer has alpha.
Correct solution is to add one more blending step and mix PIPE0 with
background, but it is not supported by the hardware.

Use premultiplied alpha buffer of PIPE0 overlay channel as is.
In this case we got same effect as mixing PIPE0 with black background.

Signed-off-by: Roman Stratiienko 

---

v4:
- Initial version, depends on other unmerged patches in the patchset.
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index dd6145f80c36..d94f4d8b9128 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -106,7 +106,7 @@ static void sun8i_ui_layer_update_alpha(struct sun8i_mixer 
*mixer, int channel,
regmap_update_bits(mixer->engine.regs,
   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
-  SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));
+  zpos ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
 }
 
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index e6d8a539614f..68a6843db4ab 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -108,7 +108,7 @@ static void sun8i_vi_layer_update_alpha(struct sun8i_mixer 
*mixer, int channel,
regmap_update_bits(mixer->engine.regs,
   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
-  (mixer->cfg->is_de3) ?
+  (zpos != 0 && mixer->cfg->is_de3) ?
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
 
 }
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


.[PATCH v4 0/4] drm/sun4i: Improve alpha processing

2020-03-03 Thread Roman Stratiienko
Patches 1-2 already reviewed and ready to be applied.

Patch 4 is RFC and require more testing on real hardware.

[PATCH v4 1/4] drm/sun4i: Add alpha property for sun8i UI layer
[PATCH v4 2/4] drm/sun4i: Add alpha property for sun8i and sun50i VI
[PATCH v4 3/4] drm/sun4i: Add support for premulti/coverage blending
[PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer

 drivers/gpu/drm/sun4i/sun8i_mixer.h|  2 +
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 50 
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 10 
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 72 +---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 16 ++
 5 files changed, 142 insertions(+), 8 deletions(-)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4 2/4] drm/sun4i: Add alpha property for sun8i and sun50i VI layer

2020-03-03 Thread Roman Stratiienko
DE3.0 VI layers supports plane-global alpha channel.
DE2.0 FCC block have GLOBAL_ALPHA register that can be used as alpha source
for blender.

Add alpha property to the DRM plane and connect it to the
corresponding registers in the mixer.

Do not add alpha property for V3s SOC that have DE2.0 and 2 VI planes.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 

---
v2: Initial version by mistake
v3:
- Skip adding & applying alpha property if VI count > 1
V4:
- Changed author e-mail address to avoid mail rejecting
- Picked reviewed-by line
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 48 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 ++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 2344938be3e4..f2469b5e97ee 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -65,6 +65,36 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   if (mixer->cfg->is_de3) {
+   mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
+  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+   val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
+   (plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
+ overlay),
+  mask, val);
+   } else if (mixer->cfg->vi_num == 1) {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA
+   (plane->state->alpha >> 8));
+   }
+}
+
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -248,14 +278,6 @@ static int sun8i_vi_layer_update_formats(struct 
sun8i_mixer *mixer, int channel,
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 
-   /* It seems that YUV formats use global alpha setting. */
-   if (mixer->cfg->is_de3)
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
- overlay),
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK,
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(0xff));
-
return 0;
 }
 
@@ -373,6 +395,8 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_vi_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_vi_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_vi_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -472,6 +496,14 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   if (mixer->cfg->vi_num == 1 || mixer->cfg->is_de3) {
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+   }
+
ret = drm_plane_create_zpos_property(>plane, index,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index eaa6076f5dbc..48c399e1c86d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -29,14 +29,25 @@
 #define SUN8I_MIXER_CHAN_VI_VDS_UV(base) \
((base) 

[PATCH v4 1/4] drm/sun4i: Add alpha property for sun8i UI layer

2020-03-03 Thread Roman Stratiienko
DE2.0 and DE3.0 UI layers supports plane-global alpha channel.
Add alpha property to the DRM plane and connect it to the
corresponding registers in mixer.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
v2: Initial commit by mistake
v3:
- Picked `reviewed-by` line
V4:
- Changed author e-mail to avoid mail rejecting.
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 19f42004cebe..5278032567a3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,6 +72,27 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+
+   val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+  mask, val);
+}
+
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -258,6 +279,8 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_ui_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_ui_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -332,6 +355,12 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+
ret = drm_plane_create_zpos_property(>plane, channel,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
index f4ab1cf6cded..e3e32ee1178d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -40,6 +40,11 @@
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK  GENMASK(12, 8)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET8
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK  GENMASK(31, 24)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(x)((x) << 24)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL((0) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER((1) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
 
 struct sun8i_mixer;
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/2] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

2020-01-04 Thread Roman Stratiienko
чт, 2 янв. 2020 г., 09:42 Jernej Škrabec :
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiie...@globallogic.com napisal(a):
> > From: Roman Stratiienko 
> >
> > 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 
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec 
>
> 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

Re: [PATCH v3 2/2] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

2020-01-04 Thread Roman Stratiienko
чт, 2 янв. 2020 г., 12:08 Maxime Ripard :
>
> Hi,
>
> On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiie...@globallogic.com 
> wrote:
> > From: Roman Stratiienko 
> >
> > 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 
>
> 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 1/2] drm/sun4i: Add alpha property for sun8i UI layer

2020-01-04 Thread roman . stratiienko
From: Roman Stratiienko 

DE2.0 and DE3.0 UI layers supports plane-global alpha channel.
Add alpha property to the DRM plane and connect it to the
corresponding registers in mixer.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
v2: Initial commit by mistake
v3:
- Picked `reviewed-by` line
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index c87fd842918e..4343ea9f8cf8 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,6 +72,27 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+
+   val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+  mask, val);
+}
+
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -288,6 +309,8 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_ui_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_ui_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -365,6 +388,12 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+
ret = drm_plane_create_zpos_property(>plane, channel,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
index f4ab1cf6cded..e3e32ee1178d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -40,6 +40,11 @@
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK  GENMASK(12, 8)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET8
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK  GENMASK(31, 24)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(x)((x) << 24)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL((0) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER((1) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
 
 struct sun8i_mixer;
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 2/2] drm/sun4i: Add alpha property for sun8i and sun50i VI layer

2020-01-04 Thread roman . stratiienko
From: Roman Stratiienko 

DE3.0 VI layers supports plane-global alpha channel.
DE2.0 FCC block have GLOBAL_ALPHA register that can be used as alpha source
for blender.

Add alpha property to the DRM plane and connect it to the
corresponding registers in the mixer.

Do not add alpha property for systems with DE2.0 and more than 1 VI planes.

Signed-off-by: Roman Stratiienko 
---
v2: Initial version by mistake
v3:
- Skip adding & applying alpha property if VI count > 1
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 48 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 ++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 42d445d23773..e61aec7d6d07 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -65,6 +65,36 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   if (mixer->cfg->is_de3) {
+   mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
+  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+   val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
+   (plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
+ overlay),
+  mask, val);
+   } else if (mixer->cfg->vi_num == 1) {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA
+   (plane->state->alpha >> 8));
+   }
+}
+
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -248,14 +278,6 @@ static int sun8i_vi_layer_update_formats(struct 
sun8i_mixer *mixer, int channel,
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 
-   /* It seems that YUV formats use global alpha setting. */
-   if (mixer->cfg->is_de3)
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
- overlay),
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK,
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(0xff));
-
return 0;
 }
 
@@ -373,6 +395,8 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_vi_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_vi_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_vi_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -464,6 +488,14 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   if (mixer->cfg->vi_num == 1 || mixer->cfg->is_de3) {
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+   }
+
ret = drm_plane_create_zpos_property(>plane, index,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index eaa6076f5dbc..48c399e1c86d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -29,14 +29,25 @@
 #define SUN8I_MIXER_CHAN_VI_VDS_UV(base) \
((base) + 0xfc)
 
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG \
+   (0xAA000 

[PATCH v3 2/2] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

2020-01-02 Thread roman . stratiienko
From: Roman Stratiienko 

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 
---
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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 1/2] drm/sun4i: Add mode_set callback to the engine

2020-01-02 Thread roman . stratiienko
From: Roman Stratiienko 

Create callback to update engine's registers on mode change.

Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-31 Thread Roman Stratiienko
Please HOLD this until v3.
I forgot about blender channel frame coords that updated according to
zpos and can have gap between channels.
I will also try to simplify this patch.

On Sun, Dec 29, 2019 at 6:28 PM  wrote:
>
> From: Roman Stratiienko 
>
> To set blending channel order register software needs to know state and
> position of each channel, which impossible at plane commit stage.
>
> Move this procedure to atomic_flush stage, where all necessary information
> is available.
>
> Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2")
> Signed-off-by: Roman Stratiienko 
> ---
> v2:
> - Use SUN8I_MIXER_MAX_LAYERS macro
> - Use plane_cnt instead of hard-coded number
> - Put initialization of channel_zpos into for loop
> - Add 'Fixes' line to the commit message
> - Minor clean-ups
> - Comments added
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c| 52 +-
>  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 | 39 +++
>  4 files changed, 66 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index 8b803eb903b8..d306ad5dc093 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
> format)
>
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
> -   DRM_DEBUG_DRIVER("Committing changes\n");
> +   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +   u32 base = sun8i_blender_base(mixer);
> +   int i, j = 0;
> +   int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];
> +   u32 route = 0, pipe_ctl = 0;
> +   int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
> +
> +   DRM_DEBUG_DRIVER("Update blender routing\n");
> +   /* Assume that values in mixer->channel_zpos[] are unique except -1 */
> +
> +   for (i = 0; i < plane_cnt; i++)
> +   channel_by_zpos[i] = -1;
>
> +   /* Sort by zpos */
> +   for (i = 0; i < plane_cnt; i++) {
> +   int zpos = mixer->channel_zpos[i];
> +
> +   if (zpos >= 0 && zpos < plane_cnt)
> +   channel_by_zpos[zpos] = i;
> +   }
> +
> +   /* Route enabled blending channels first */
> +   for (i = 0; i < plane_cnt; i++) {
> +   int ch = channel_by_zpos[i];
> +
> +   if (ch >= 0) {
> +   pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
> +   route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> +   j++;
> +   }
> +   }
> +
> +   /* Set remaining routing fields to match disabled channel indices */
> +   for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < SUN8I_MIXER_MAX_LAYERS;
> +i++) {
> +   if (mixer->channel_zpos[i] < 0) {
> +   route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> +   j++;
> +   }
> +   }
> +
> +   regmap_update_bits(mixer->engine.regs, 
> SUN8I_MIXER_BLEND_PIPE_CTL(base),
> +  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, pipe_ctl);
> +
> +   regmap_write(mixer->engine.regs,
> +SUN8I_MIXER_BLEND_ROUTE(base), route);
> +
> +   DRM_DEBUG_DRIVER("Committing changes\n");
> regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF,
>  SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
>  }
> @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct 
> device *master,
>  SUN8I_MIXER_BLEND_COLOR_BLACK);
>
> plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
> -   for (i = 0; i < plane_cnt; i++)
> +   for (i = 0; i < plane_cnt; i++) {
> +   mixer->channel_zpos[i] = -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);
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> index c6cc94057faf..b193d9d1db66 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @

Re: [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-31 Thread Roman Stratiienko
False alarm. Since DRM sets normalized ZPOS there should be no gaps
and all should work as expected.

On Mon, Dec 30, 2019 at 1:25 PM Roman Stratiienko
 wrote:
>
> Please HOLD this until v3.
> I forgot about blender channel frame coords that updated according to
> zpos and can have gap between channels.
> I will also try to simplify this patch.
>
> On Sun, Dec 29, 2019 at 6:28 PM  wrote:
> >
> > From: Roman Stratiienko 
> >
> > To set blending channel order register software needs to know state and
> > position of each channel, which impossible at plane commit stage.
> >
> > Move this procedure to atomic_flush stage, where all necessary information
> > is available.
> >
> > Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2")
> > Signed-off-by: Roman Stratiienko 
> > ---
> > v2:
> > - Use SUN8I_MIXER_MAX_LAYERS macro
> > - Use plane_cnt instead of hard-coded number
> > - Put initialization of channel_zpos into for loop
> > - Add 'Fixes' line to the commit message
> > - Minor clean-ups
> > - Comments added
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 52 +-
> >  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 | 39 +++
> >  4 files changed, 66 insertions(+), 72 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > index 8b803eb903b8..d306ad5dc093 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
> > format)
> >
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> > -   DRM_DEBUG_DRIVER("Committing changes\n");
> > +   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +   u32 base = sun8i_blender_base(mixer);
> > +   int i, j = 0;
> > +   int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];
> > +   u32 route = 0, pipe_ctl = 0;
> > +   int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
> > +
> > +   DRM_DEBUG_DRIVER("Update blender routing\n");
> > +   /* Assume that values in mixer->channel_zpos[] are unique except -1 
> > */
> > +
> > +   for (i = 0; i < plane_cnt; i++)
> > +   channel_by_zpos[i] = -1;
> >
> > +   /* Sort by zpos */
> > +   for (i = 0; i < plane_cnt; i++) {
> > +   int zpos = mixer->channel_zpos[i];
> > +
> > +   if (zpos >= 0 && zpos < plane_cnt)
> > +   channel_by_zpos[zpos] = i;
> > +   }
> > +
> > +   /* Route enabled blending channels first */
> > +   for (i = 0; i < plane_cnt; i++) {
> > +   int ch = channel_by_zpos[i];
> > +
> > +   if (ch >= 0) {
> > +   pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
> > +   route |= ch << 
> > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > +   j++;
> > +   }
> > +   }
> > +
> > +   /* Set remaining routing fields to match disabled channel indices */
> > +   for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < 
> > SUN8I_MIXER_MAX_LAYERS;
> > +i++) {
> > +   if (mixer->channel_zpos[i] < 0) {
> > +   route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > +   j++;
> > +   }
> > +   }
> > +
> > +   regmap_update_bits(mixer->engine.regs, 
> > SUN8I_MIXER_BLEND_PIPE_CTL(base),
> > +  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, pipe_ctl);
> > +
> > +   regmap_write(mixer->engine.regs,
> > +SUN8I_MIXER_BLEND_ROUTE(base), route);
> > +
> > +   DRM_DEBUG_DRIVER("Committing changes\n");
> > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF,
> >  SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
> >  }
> > @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, 
> > struct device *master,
> >  SUN8I_MIXER_BLEND_COLOR_BLACK);
> >
> > plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
> > -   for (i = 0; i < plane_cnt; i++)
> > + 

[PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

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: 90212fffa4fc ("drm/sun4i: Use values calculated by atomic check")
Signed-off-by: Roman Stratiienko 
---
v2:
- Split commit in 2 parts
- Add Fixes line to the commit message
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 35 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --
 2 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index d306ad5dc093..5d90a95ff855 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -257,6 +257,40 @@ 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 dst_w = mode->crtc_hdisplay;
+   u32 dst_h = mode->crtc_vdisplay;
+   u32 outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
+   bool interlaced = false;
+   u32 val;
+   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+   u32 bld_base = sun8i_blender_base(mixer);
+
+   DRM_DEBUG_DRIVER("Mode change, 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);
+
+   interlaced = 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");
+}
+
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
@@ -356,6 +390,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 ee7c13d8710f..23c2f4b68c89 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,36 +72,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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 2/2] drm/sun4i: Add alpha property for sun8i and sun50i VI layer

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

DE3.0 VI layers supports plane-global alpha channel.
DE2.0 FCC block have GLOBAL_ALPHA register that can be used as alpha source
for blender.

Add alpha property to the DRM plane and connect it to the
corresponding registers in the mixer.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 46 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 ++
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 42d445d23773..db32a78c75d9 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -65,6 +65,36 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   if (mixer->cfg->is_de3) {
+   mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
+  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+   val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
+   (plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
+ overlay),
+  mask, val);
+   } else {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
+  SUN8I_MIXER_FCC_GLOBAL_ALPHA
+   (plane->state->alpha >> 8));
+   }
+}
+
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -248,14 +278,6 @@ static int sun8i_vi_layer_update_formats(struct 
sun8i_mixer *mixer, int channel,
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 
-   /* It seems that YUV formats use global alpha setting. */
-   if (mixer->cfg->is_de3)
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
- overlay),
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK,
-  SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(0xff));
-
return 0;
 }
 
@@ -373,6 +395,8 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_vi_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_vi_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_vi_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -464,6 +488,12 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+
ret = drm_plane_create_zpos_property(>plane, index,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index eaa6076f5dbc..48c399e1c86d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -29,14 +29,25 @@
 #define SUN8I_MIXER_CHAN_VI_VDS_UV(base) \
((base) + 0xfc)
 
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG \
+   (0xAA000 + 0x90)
+
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA(x)((x) << 24)
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK  GENMASK(31, 24)
+
 #define SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN  BIT(0)
 /* RGB mode should be set for RGB formats and cleared for YCbCr */
 #define SUN8I_MIXER_CHAN_VI_

[PATCH v2 1/2] drm/sun4i: Add alpha property for sun8i UI layer

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

DE2.0 and DE3.0 UI layers supports plane-global alpha channel.
Add alpha property to the DRM plane and connect it to the
corresponding registers in mixer.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index c87fd842918e..4343ea9f8cf8 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,6 +72,27 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
}
 }
 
+static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+   int overlay, struct drm_plane *plane)
+{
+   u32 mask, val, ch_base;
+
+   ch_base = sun8i_channel_base(mixer, channel);
+
+   mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+
+   val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
+
+   val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+  mask, val);
+}
+
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
   int overlay, struct drm_plane *plane,
   unsigned int zpos)
@@ -288,6 +309,8 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+   sun8i_ui_layer_update_alpha(mixer, layer->channel,
+   layer->overlay, plane);
sun8i_ui_layer_update_formats(mixer, layer->channel,
  layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -365,6 +388,12 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
 
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+   ret = drm_plane_create_alpha_property(>plane);
+   if (ret) {
+   dev_err(drm->dev, "Couldn't add alpha property\n");
+   return ERR_PTR(ret);
+   }
+
ret = drm_plane_create_zpos_property(>plane, channel,
 0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
index f4ab1cf6cded..e3e32ee1178d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -40,6 +40,11 @@
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK  GENMASK(12, 8)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET8
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK  GENMASK(31, 24)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(x)((x) << 24)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL((0) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER((1) << 
1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
 
 struct sun8i_mixer;
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

To set blending channel order register software needs to know state and
position of each channel, which impossible at plane commit stage.

Move this procedure to atomic_flush stage, where all necessary information
is available.

Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2")
Signed-off-by: Roman Stratiienko 
---
v2:
- Use SUN8I_MIXER_MAX_LAYERS macro
- Use plane_cnt instead of hard-coded number
- Put initialization of channel_zpos into for loop
- Add 'Fixes' line to the commit message
- Minor clean-ups
- Comments added
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 52 +-
 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 | 39 +++
 4 files changed, 66 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 8b803eb903b8..d306ad5dc093 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
format)
 
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
-   DRM_DEBUG_DRIVER("Committing changes\n");
+   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+   u32 base = sun8i_blender_base(mixer);
+   int i, j = 0;
+   int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];
+   u32 route = 0, pipe_ctl = 0;
+   int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
+
+   DRM_DEBUG_DRIVER("Update blender routing\n");
+   /* Assume that values in mixer->channel_zpos[] are unique except -1 */
+
+   for (i = 0; i < plane_cnt; i++)
+   channel_by_zpos[i] = -1;
 
+   /* Sort by zpos */
+   for (i = 0; i < plane_cnt; i++) {
+   int zpos = mixer->channel_zpos[i];
+
+   if (zpos >= 0 && zpos < plane_cnt)
+   channel_by_zpos[zpos] = i;
+   }
+
+   /* Route enabled blending channels first */
+   for (i = 0; i < plane_cnt; i++) {
+   int ch = channel_by_zpos[i];
+
+   if (ch >= 0) {
+   pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
+   route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
+   j++;
+   }
+   }
+
+   /* Set remaining routing fields to match disabled channel indices */
+   for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < SUN8I_MIXER_MAX_LAYERS;
+i++) {
+   if (mixer->channel_zpos[i] < 0) {
+   route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
+   j++;
+   }
+   }
+
+   regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, pipe_ctl);
+
+   regmap_write(mixer->engine.regs,
+SUN8I_MIXER_BLEND_ROUTE(base), route);
+
+   DRM_DEBUG_DRIVER("Committing changes\n");
regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF,
 SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
 }
@@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
 SUN8I_MIXER_BLEND_COLOR_BLACK);
 
plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
-   for (i = 0; i < plane_cnt; i++)
+   for (i = 0; i < plane_cnt; i++) {
+   mixer->channel_zpos[i] = -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);
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index c6cc94057faf..b193d9d1db66 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -13,6 +13,8 @@
 #include "sun8i_csc.h"
 #include "sunxi_engine.h"
 
+#define SUN8I_MIXER_MAX_LAYERS 5
+
 #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1))
 #define SUN8I_MIXER_COORD(x, y)((y) << 16 | (x))
 
@@ -176,6 +178,9 @@ struct sun8i_mixer {
 
struct clk  *bus_clk;
struct clk  *mod_clk;
+
+   /* -1 means that layer is disabled */
+   int channel_zpos[SUN8I_MIXER_MAX_LAYERS];
 };
 
 static inline struct sun8i_mixer *
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index c87fd842918e..ee7c13d8710f 100644
--- a/drivers/gpu/drm/sun4i/sun

[PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

Create callback to update engine's registers on mode change.

Signed-off-by: Roman Stratiienko 
---
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)
---
 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..44102783ee3c 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 other mode attributes.
+*
+* This function is optional.
+*/
+   void (*mode_set)(struct sunxi_engine *engine,
+struct drm_display_mode *mode);
 };
 
 /**
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 4/4] drm/sun4i: Update mixer's internal registers after initialization

2019-12-31 Thread roman . stratiienko
From: Roman Stratiienko 

At system start blink of u-boot ghost framebuffer can be observed.
Fix it.

Fixes: 9d75b8c0b999 ("drm/sun4i: add support for Allwinner DE2 mixers")
Signed-off-by: Roman Stratiienko 
Reviewed-by: Jernej Skrabec 
---
v2:
- Picked 'Reviewed-by' line
- Added 'Fixes' line
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 5d90a95ff855..86711d8a9c84 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -576,6 +576,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
   SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
 
+   regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_DBUFF,
+SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
+
return 0;
 
 err_disable_bus_clk:
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC 3/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-29 Thread Roman Stratiienko
Hello Jernej,

Thank you for review.

On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec  wrote:
>
> Hi!
>
> Dne sobota, 28. december 2019 ob 21:28:17 CET je
> roman.stratiie...@globallogic.com napisal(a):
> > From: Roman Stratiienko 
> >
> > To set blending channel order register software needs to know state and
> > position of each channel, which impossible at plane commit stage.
> >
> > Move this procedure to atomic_flush stage, where all necessary information
> > is available.
> >
> > Signed-off-by: Roman Stratiienko 
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 47 +-
> >  drivers/gpu/drm/sun4i/sun8i_mixer.h|  3 ++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ---
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++--
> >  4 files changed, 60 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine
> > *engine,
> >
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> > - DRM_DEBUG_DRIVER("Committing changes\n");
> > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > + u32 base = sun8i_blender_base(mixer);
> > + int i, j;
> > + int channel_by_zpos[4] = {-1, -1, -1, -1};
> > + u32 route = 0, pipe_ctl = 0;
> > +
> > + DRM_DEBUG_DRIVER("Update blender routing\n");
>
> Use drm_dbg().
>
> > + for (i = 0; i < 4; i++) {
> > + int zpos = mixer->channel_zpos[i];
>
> channel_zpos can hold 5 elements which is also theoretical maximum for current
> HW design. Why do you check only 4 elements?
>

I'll use plane_cnt as it done in mixer_bind

> It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so everyone
> would understand where this number comes from.

Will do.

>
> > +
> > + if (zpos >= 0 && zpos < 4)
> > + channel_by_zpos[zpos] = i;
> > + }
> > +
> > + j = 0;
> > + for (i = 0; i < 4; i++) {
> > + int ch = channel_by_zpos[i];
> > +
> > + if (ch >= 0) {
> > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
> > + route |= ch <<
> SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > + j++;
> > + }
> > + }
> > +
> > + for (i = 0; i < 4 && j < 4; i++) {
> > + int zpos = mixer->channel_zpos[i];
> >
> > + if (zpos < 0) {
> > + route |= i <<
> SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > + j++;
> > + }
> > + }
> > +
> > + regmap_update_bits(mixer->engine.regs,
> SUN8I_MIXER_BLEND_PIPE_CTL(base),
> > +SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK,
> pipe_ctl);
> > +
> > + regmap_write(mixer->engine.regs,
> > +  SUN8I_MIXER_BLEND_ROUTE(base), route);
> > +
> > + DRM_DEBUG_DRIVER("Committing changes\n");
>
> Use drm_dbg().

According to 
https://github.com/torvalds/linux/commit/99a954874e7b9f0c8058476575593b3beb5731a5#diff-b0cd2d683c6afbab7bd54173cfd3d3ecR289
,
DRM_DEBUG_DRIVER uses drm_dbg.
Also, using drm_dbg with category macro would require larger indent,
making harder to fit in 80 chars limit.
Are there any plans to deprecate DRM_DEBUG_DRIVER macro?

>
> >   regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF,
> >SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
> >  }
> > @@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, struct
> > device *master, mixer->engine.ops = _engine_ops;
> >   mixer->engine.node = dev->of_node;
> >
> > + mixer->channel_zpos[0] = -1;
> > + mixer->channel_zpos[1] = -1;
> > + mixer->channel_zpos[2] = -1;
> > + mixer->channel_zpos[3] = -1;
> > + mixer->channel_zpos[4] = -1;
> > +
>
> for loop would be better, especially using proposed macro.

I'll put it into already existent for-loop below.

>
> Best regards,
> Jernej
>
> >   /*
> >* While this function can fail, we shouldn't do anything
> >* if this happens. Some early DE2 DT entries do

[RFC 2/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.

2019-12-29 Thread roman . stratiienko
From: Roman Stratiienko 

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.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun4i_crtc.c |  4 +++
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 35 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --
 drivers/gpu/drm/sun4i/sunxi_engine.h   |  8 ++
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c 
b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index 3a153648b369..156ea8f19d7d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -139,8 +139,12 @@ static void sun4i_crtc_mode_set_nofb(struct drm_crtc *crtc)
struct drm_display_mode *mode = >state->adjusted_mode;
struct drm_encoder *encoder = sun4i_crtc_get_encoder(crtc);
struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
+   struct sunxi_engine *engine = scrtc->engine;
 
sun4i_tcon_mode_set(scrtc->tcon, encoder, mode);
+
+   if (engine->ops->mode_set)
+   engine->ops->mode_set(engine, mode);
 }
 
 static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index eea4813602b7..bb9a665fd053 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -257,6 +257,40 @@ 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 dst_w = mode->crtc_hdisplay;
+   u32 dst_h = mode->crtc_vdisplay;
+   u32 outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
+   bool interlaced = false;
+   u32 val;
+   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+   u32 bld_base = sun8i_blender_base(mixer);
+
+   DRM_DEBUG_DRIVER("Mode change, 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);
+
+   interlaced = 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");
+}
+
 static void sun8i_atomic_begin(struct sunxi_engine *engine,
   struct drm_crtc_state *old_state)
 {
@@ -325,6 +359,7 @@ static const struct sunxi_engine_ops sun8i_engine_ops = {
.commit = sun8i_mixer_commit,
.layers_init= sun8i_layers_init,
.atomic_begin   = sun8i_atomic_begin,
+   .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 c87fd842918e..893076716070 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -99,36 +99,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_INTE

Re: [RFC 3/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-29 Thread Roman Stratiienko
My proposal is to go with DRM_DEBUG_DRIVER for now.
In case stable branches would be interested in these fixes, it will be
easier to backport.

Also I am using v5.4 for testing since Google AOSP patches does not
work correctly with mainline and missing for kernel-next.
Maintaining 2 variants will be much harder for me.

On Sun, Dec 29, 2019 at 3:02 PM Jernej Škrabec  wrote:
>
> Dne nedelja, 29. december 2019 ob 13:47:38 CET je Roman Stratiienko
> napisal(a):
> > On Sun, Dec 29, 2019 at 2:18 PM Jernej Škrabec 
> wrote:
> > > Dne nedelja, 29. december 2019 ob 13:08:19 CET je Roman Stratiienko
> > >
> > > napisal(a):
> > > > Hello Jernej,
> > > >
> > > > Thank you for review.
> > > >
> > > > On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec
> > > > 
> > >
> > > wrote:
> > > > > Hi!
> > > > >
> > > > > Dne sobota, 28. december 2019 ob 21:28:17 CET je
> > > > >
> > > > > roman.stratiie...@globallogic.com napisal(a):
> > > > > > From: Roman Stratiienko 
> > > > > >
> > > > > > To set blending channel order register software needs to know state
> > > > > > and
> > > > > > position of each channel, which impossible at plane commit stage.
> > > > > >
> > > > > > Move this procedure to atomic_flush stage, where all necessary
> > > > > > information
> > > > > > is available.
> > > > > >
> > > > > > Signed-off-by: Roman Stratiienko 
> > > > > > ---
> > > > > >
> > > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 47
> > > > > >  +-
> > > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.h|  3 ++
> > > > > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ---
> > > > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++--
> > > > > >  4 files changed, 60 insertions(+), 71 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index
> > > > > > bb9a665fd053..da84fccf7784
> > > > > > 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > > > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct
> > > > > > sunxi_engine
> > > > > > *engine,
> > > > > >
> > > > > >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> > > > > >  {
> > > > > >
> > > > > > - DRM_DEBUG_DRIVER("Committing changes\n");
> > > > > > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > > > > > + u32 base = sun8i_blender_base(mixer);
> > > > > > + int i, j;
> > > > > > + int channel_by_zpos[4] = {-1, -1, -1, -1};
> > > > > > + u32 route = 0, pipe_ctl = 0;
> > > > > > +
> > > > > > + DRM_DEBUG_DRIVER("Update blender routing\n");
> > > > >
> > > > > Use drm_dbg().
> > > > >
> > > > > > + for (i = 0; i < 4; i++) {
> > > > > > + int zpos = mixer->channel_zpos[i];
> > > > >
> > > > > channel_zpos can hold 5 elements which is also theoretical maximum for
> > > > > current HW design. Why do you check only 4 elements?
> > > >
> > > > I'll use plane_cnt as it done in mixer_bind
> > > >
> > > > > It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so
> > > > > everyone would understand where this number comes from.
> > > >
> > > > Will do.
> > > >
> > > > > > +
> > > > > > + if (zpos >= 0 && zpos < 4)
> > > > > > + channel_by_zpos[zpos] = i;
> > > > > > + }
> > > > > > +
> > > > > > + j = 0;
> > > > > > + for (i = 0; i < 4; i++) {
> > > > > > + int ch = channel_by_zpos[i];
> > > > > > +
> > > > > > + if (ch >= 0) {
> > > > &g

[RFC 4/4] drm/sun4i: Update mixer's internal registers after initialization

2019-12-29 Thread roman . stratiienko
From: Roman Stratiienko 

At system start blink of u-boot ghost framebuffer can be observed.
Fix it.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index da84fccf7784..b906b8cc464e 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -588,6 +588,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
   SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
 
+   regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_DBUFF,
+SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
+
return 0;
 
 err_disable_bus_clk:
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC 3/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-29 Thread roman . stratiienko
From: Roman Stratiienko 

To set blending channel order register software needs to know state and
position of each channel, which impossible at plane commit stage.

Move this procedure to atomic_flush stage, where all necessary information
is available.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 47 +-
 drivers/gpu/drm/sun4i/sun8i_mixer.h|  3 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++--
 4 files changed, 60 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index bb9a665fd053..da84fccf7784 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine *engine,
 
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
-   DRM_DEBUG_DRIVER("Committing changes\n");
+   struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+   u32 base = sun8i_blender_base(mixer);
+   int i, j;
+   int channel_by_zpos[4] = {-1, -1, -1, -1};
+   u32 route = 0, pipe_ctl = 0;
+
+   DRM_DEBUG_DRIVER("Update blender routing\n");
+   for (i = 0; i < 4; i++) {
+   int zpos = mixer->channel_zpos[i];
+
+   if (zpos >= 0 && zpos < 4)
+   channel_by_zpos[zpos] = i;
+   }
+
+   j = 0;
+   for (i = 0; i < 4; i++) {
+   int ch = channel_by_zpos[i];
+
+   if (ch >= 0) {
+   pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
+   route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
+   j++;
+   }
+   }
+
+   for (i = 0; i < 4 && j < 4; i++) {
+   int zpos = mixer->channel_zpos[i];
 
+   if (zpos < 0) {
+   route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
+   j++;
+   }
+   }
+
+   regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, pipe_ctl);
+
+   regmap_write(mixer->engine.regs,
+SUN8I_MIXER_BLEND_ROUTE(base), route);
+
+   DRM_DEBUG_DRIVER("Committing changes\n");
regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF,
 SUN8I_MIXER_GLOBAL_DBUFF_ENABLE);
 }
@@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
mixer->engine.ops = _engine_ops;
mixer->engine.node = dev->of_node;
 
+   mixer->channel_zpos[0] = -1;
+   mixer->channel_zpos[1] = -1;
+   mixer->channel_zpos[2] = -1;
+   mixer->channel_zpos[3] = -1;
+   mixer->channel_zpos[4] = -1;
+
/*
 * While this function can fail, we shouldn't do anything
 * if this happens. Some early DE2 DT entries don't provide
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 915479cc3077..9c2ff87923d8 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -178,6 +178,9 @@ struct sun8i_mixer {
 
struct clk  *bus_clk;
struct clk  *mod_clk;
+
+   /* -1 means that layer is disabled */
+   int channel_zpos[5];
 };
 
 static inline struct sun8i_mixer *
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 893076716070..23c2f4b68c89 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"
 
 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;
 
-   bld_base = sun8i_blender_base(mixer);
ch_base = sun8i_channel_base(mixer, channel);
 
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);
 
-   if (!enable || zpos != old_zpos) {
-   regmap_update_bits(mixer->engine.regs,
-  SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
-  SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
-  

[RFC 1/4] drm/sun4i: Wait for previous mixing process finish before committing next

2019-12-29 Thread roman . stratiienko
From: Roman Stratiienko 

Screen composition that requires dynamic layout modification,
especially scaling is corrupted when layout changes.

For example if one of the layer scales down, misaligned lines can be
observed, and dynamic increasing of destination area makes mixer to hang
and draw nothing after drawing modified layer.

After deep investigation it turns that scaler double-buffered registers
are not latched by GLB_DBUFFER bit, instead thay are latched immidiately.

Only way to avoid artifacts is to change the registers after mixer finish
previous frame.

Similar was made in sunxi BSP - scaler register values was stored into RAM,
and moved into registers at sync together with GLB_DBUFFER.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++
 drivers/gpu/drm/sun4i/sun8i_mixer.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 8b803eb903b8..eea4813602b7 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -257,6 +257,20 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
format)
return NULL;
 }
 
+static void sun8i_atomic_begin(struct sunxi_engine *engine,
+  struct drm_crtc_state *old_state)
+{
+   int reg, ret;
+
+   ret = regmap_read_poll_timeout(engine->regs, SUN8I_MIXER_GLOBAL_STATUS,
+  reg,
+  !(reg & SUN8I_MIXER_GLOBAL_STATUS_BUSY),
+  200, 10);
+
+   if (ret)
+   pr_warn("%s: Wait for frame finish timeout\n", __func__);
+}
+
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
DRM_DEBUG_DRIVER("Committing changes\n");
@@ -310,6 +324,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,
+   .atomic_begin   = sun8i_atomic_begin,
 };
 
 static struct regmap_config sun8i_mixer_regmap_config = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h 
b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index c6cc94057faf..915479cc3077 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -25,6 +25,8 @@
 
 #define SUN8I_MIXER_GLOBAL_DBUFF_ENABLEBIT(0)
 
+#define SUN8I_MIXER_GLOBAL_STATUS_BUSY BIT(4)
+
 #define DE2_MIXER_UNIT_SIZE0x6000
 #define DE3_MIXER_UNIT_SIZE0x3000
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC 3/4] drm/sun4i: Reimplement plane z position setting logic

2019-12-29 Thread Roman Stratiienko
On Sun, Dec 29, 2019 at 2:18 PM Jernej Škrabec  wrote:
>
> Dne nedelja, 29. december 2019 ob 13:08:19 CET je Roman Stratiienko
> napisal(a):
> > Hello Jernej,
> >
> > Thank you for review.
> >
> > On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec 
> wrote:
> > > Hi!
> > >
> > > Dne sobota, 28. december 2019 ob 21:28:17 CET je
> > >
> > > roman.stratiie...@globallogic.com napisal(a):
> > > > From: Roman Stratiienko 
> > > >
> > > > To set blending channel order register software needs to know state and
> > > > position of each channel, which impossible at plane commit stage.
> > > >
> > > > Move this procedure to atomic_flush stage, where all necessary
> > > > information
> > > > is available.
> > > >
> > > > Signed-off-by: Roman Stratiienko 
> > > > ---
> > > >
> > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c| 47 +-
> > > >  drivers/gpu/drm/sun4i/sun8i_mixer.h|  3 ++
> > > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ---
> > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++--
> > > >  4 files changed, 60 insertions(+), 71 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784
> > > > 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > > > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine
> > > > *engine,
> > > >
> > > >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> > > >  {
> > > >
> > > > - DRM_DEBUG_DRIVER("Committing changes\n");
> > > > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > > > + u32 base = sun8i_blender_base(mixer);
> > > > + int i, j;
> > > > + int channel_by_zpos[4] = {-1, -1, -1, -1};
> > > > + u32 route = 0, pipe_ctl = 0;
> > > > +
> > > > + DRM_DEBUG_DRIVER("Update blender routing\n");
> > >
> > > Use drm_dbg().
> > >
> > > > + for (i = 0; i < 4; i++) {
> > > > + int zpos = mixer->channel_zpos[i];
> > >
> > > channel_zpos can hold 5 elements which is also theoretical maximum for
> > > current HW design. Why do you check only 4 elements?
> >
> > I'll use plane_cnt as it done in mixer_bind
> >
> > > It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so
> > > everyone would understand where this number comes from.
> >
> > Will do.
> >
> > > > +
> > > > + if (zpos >= 0 && zpos < 4)
> > > > + channel_by_zpos[zpos] = i;
> > > > + }
> > > > +
> > > > + j = 0;
> > > > + for (i = 0; i < 4; i++) {
> > > > + int ch = channel_by_zpos[i];
> > > > +
> > > > + if (ch >= 0) {
> > > > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j);
> > > > + route |= ch <<
> > >
> > > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > >
> > > > + j++;
> > > > + }
> > > > + }
> > > > +
> > > > + for (i = 0; i < 4 && j < 4; i++) {
> > > > + int zpos = mixer->channel_zpos[i];
> > > >
> > > > + if (zpos < 0) {
> > > > + route |= i <<
> > >
> > > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
> > >
> > > > + j++;
> > > > + }
> > > > + }
> > > > +
> > > > + regmap_update_bits(mixer->engine.regs,
> > >
> > > SUN8I_MIXER_BLEND_PIPE_CTL(base),
> > >
> > > > +SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK,
> > >
> > > pipe_ctl);
> > >
> > > > +
> > > > + regmap_write(mixer->engine.regs,
> > > > +  SUN8I_MIXER_BLEND_ROUTE(base), route);
> > > > +
> > > > + DRM_DEBUG_DRIVER("Committing changes\n");
> > >
> 

Re: drm/sun4i: Add missing pixel formats to the vi layer

2019-09-20 Thread Roman Stratiienko
On Thu, Sep 19, 2019 at 9:53 PM Jernej Škrabec  wrote:
>
> Hi!
>
> Dne sreda, 18. september 2019 ob 13:05:41 CEST je
> roman.stratiie...@globallogic.com napisal(a):
> > From: Roman Stratiienko 
> >
> > According to Allwinner DE2.0 Specification REV 1.0, vi layer supports the
> > following pixel formats:  ABGR_, ARGB_, BGRA_, RGBA_
>
> It's true that DE2 VI layers support those formats, but it wouldn't change
> anything because alpha blending is not supported by those planes. These
> formats were deliberately left out because their counterparts without alpha
> exist, for example ABGR <-> XBGR. It would also confuse user, which
> would expect that alpha blending works if format with alpha channel is
> selected.
>
> Admittedly some formats with alpha are still reported as supported due to lack
> of their counterparts without alpha, but I'm fine with removing them for
> consistency.

Why not to replace 'A' with 'X' on all relevant formats and map them
to corresponding index marked with 'A' (that behaves as true 'X' for
vi)

>
> Best regards,
> Jernej
>
> >
> > Signed-off-by: Roman Stratiienko 
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index bd0e6a52d1d8..07c27e6a4b77
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > @@ -404,17 +404,21 @@ static const struct drm_plane_funcs
> > sun8i_vi_layer_funcs = { static const u32 sun8i_vi_layer_formats[] = {
> >   DRM_FORMAT_ABGR1555,
> >   DRM_FORMAT_ABGR,
> > + DRM_FORMAT_ABGR,
> >   DRM_FORMAT_ARGB1555,
> >   DRM_FORMAT_ARGB,
> > + DRM_FORMAT_ARGB,
> >   DRM_FORMAT_BGR565,
> >   DRM_FORMAT_BGR888,
> >   DRM_FORMAT_BGRA5551,
> >   DRM_FORMAT_BGRA,
> > + DRM_FORMAT_BGRA,
> >   DRM_FORMAT_BGRX,
> >   DRM_FORMAT_RGB565,
> >   DRM_FORMAT_RGB888,
> >   DRM_FORMAT_RGBA,
> >   DRM_FORMAT_RGBA5551,
> > + DRM_FORMAT_RGBA,
> >   DRM_FORMAT_RGBX,
> >   DRM_FORMAT_XBGR,
> >   DRM_FORMAT_XRGB,
>
>
>
>


-- 
Best regards,
Roman Stratiienko
Global Logic Inc.


Re: [PATCH] drm/sun4i: Use vi plane as primary

2019-09-20 Thread Roman Stratiienko
@Jernej Škrabec @Maxime Ripard

Thanks for your time and valuable suggestions.

Currently I would have to go away from mainline KMS to solve my
(Android) issues, and I hope to get back when I
find mainline-friendly solution for it.

Having no primary layer or zero-buffer primary layer and 4 overlays
could be a universal solution, but  I have not sufficient knowledge in
KMS to bring-up this idea.

On Fri, Sep 20, 2019 at 9:18 AM Maxime Ripard  wrote:
>
> Hi,
>
> On Thu, Sep 19, 2019 at 11:03:26PM +0300, Roman Stratiienko wrote:
> > Actually, I beleive this is True solution, and current one is wrong.  Let
> > me explain why.
> >
> > De2. 0 was designed to match Android hwcomposer hal requirements IMO.
> > You can easily agree with this conclusion by comparing Composer HAL and
> > De2. 0 hardware manuals.
> >
> > I faced at least 4 issues when try to run Android using the mainline kernel
> > sun8i mixer implementation. Current one, missing pixel formats (my previous
> > patch), missing plane alpha and rotation properties. I plan to fix it and
> > also send appropriate solution to the upstream.
> >
> > To achieve optimal UI performance Android requires at least 4 ui layers to
> > make screen composition. Current patch enables 4th plane usable.
>
> Note that you can also get 4 UI planes by enabling more than one UI
> layer per channel. You wouldn't be able to use alpha between each
> plane of a given channel, but we can use a similar trick than what we
> did for the pipes in the sun4i backend.
>
> Maxime



-- 
Best regards,
Roman Stratiienko
Global Logic Inc.


Re: drm/sun4i: Add missing pixel formats to the vi layer

2019-09-20 Thread Roman Stratiienko
On Fri, Sep 20, 2019 at 9:20 AM Maxime Ripard  wrote:
>
> On Thu, Sep 19, 2019 at 08:53:10PM +0200, Jernej Škrabec wrote:
> > Dne sreda, 18. september 2019 ob 13:05:41 CEST je
> > roman.stratiie...@globallogic.com napisal(a):
> > > From: Roman Stratiienko 
> > >
> > > According to Allwinner DE2.0 Specification REV 1.0, vi layer supports the
> > > following pixel formats:  ABGR_, ARGB_, BGRA_, RGBA_
> >
> > It's true that DE2 VI layers support those formats, but it wouldn't change
> > anything because alpha blending is not supported by those planes. These
> > formats were deliberately left out because their counterparts without alpha
> > exist, for example ABGR <-> XBGR. It would also confuse user, which
> > would expect that alpha blending works if format with alpha channel is
> > selected.
>
> I'm not too familiar with the DE2 code, but why is alpha not working
> if the VI planes support formats with alpha?

Good question. It mentioned in the datasheet
https://linux-sunxi.org/images/7/7b/Allwinner_DE2.0_Spec_V1.0.pdf
on page 95: "All ui layers' alpha is useless"
And my experiments proves it.

My assumption that vi uses post-processing that cuts out alpha values.

>
> Thanks!
> Maxime



-- 
Best regards,
Roman Stratiienko
Global Logic Inc.


Re: [PATCH] drm/sun4i: Use vi plane as primary

2019-09-20 Thread Roman Stratiienko
[ Resending  message in plain mode ]

Hello guys,

Actually, I believe this is True solution, and current one is wrong.
Let me explain why.

De2. 0 was designed to match Android hwcomposer hal requirements IMO.
You can easily agree with this conclusion by comparing Composer HAL
and De2. 0 hardware manuals.

I faced at least 4 issues when trying to run Android using the
mainline kernel sun8i mixer implementation. Current one, missing pixel
formats (my previous patch), missing plane alpha and rotation
properties. I plan to fix it and also send appropriate solution to the
upstream.

To achieve optimal UI performance Android requires at least 4 ui
layers to make screen composition. Current patch enables 4th plane
usable.

As for using vi plane to display video. I assume that some of the
current users may have regression in their software, but it could be
easily fixed. For example if vi layer isn't fullscreen and should be
on top of the other layers, it can actually be placed on the bottom
and overlayed with pictures with transparent rectangles in video
region.
But I assume most of users such as browser etc. uses GPU for that.

And if you are watching fullscreen video, I can imagine only subtitles
layer and advertising layers on top of the video layers.


On Thu, Sep 19, 2019 at 9:15 PM Jernej Škrabec  wrote:
>
> Dne četrtek, 19. september 2019 ob 19:17:54 CEST je Maxime Ripard napisal(a):
> > Hi,
> >
> > On Thu, Sep 19, 2019 at 03:37:03PM +0300, roman.stratiie...@globallogic.com
> wrote:
> > > From: Roman Stratiienko 
> > >
> > > DE2.0 blender does not take into the account alpha channel of vi layer.
> > > Thus makes overlaying of this layer totally opaque.
> > > Using vi layer as bottom solves this issue.
>
> What issue? Overlays don't have to be "full screen", thus missing support for
> alpha blending doesn't make it less valuable. And VI planes are already placed
> at the bottom (zpos = 0).
>
> > >
> > > Tested on Android.
> > >
> > > Signed-off-by: Roman Stratiienko 
> >
> > It sounds like a workaround more than an actual fix.
> >
> > If the VI planes can't use the alpha, then we should just stop
> > reporting that format.
> >
> > Jernej, what do you think?
>
> Commit message is misleading. What this commit actually does is moving primary
> plane from first UI plane to bottom most plane, i.e. first VI plane. However, 
> VI
> planes are scarce resource, almost all mixers have only one. I wouldn't set it
> as primary, because it's the only one which provide support for YUV formats.
> That could be used for example by video player for zero-copy rendering.
> Probably most apps wouldn't touch it if it was primary (that's usually
> reserved for window manager, if used).
>
> I left few formats with alpha channel exposed by VI planes, just because they
> don't have equivalent format without alpha. But I'm fine with removing them if
> you all agree on that.
>
> Best regards,
> Jernej
>
> >
> > Maxime
> >
> > > ---
> > >
> > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ---
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +-
> > >  2 files changed, 35 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index dd2a1c851939..25183badc85f
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > @@ -99,36 +99,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;
> > > 

[PATCH] drm/sun4i: Use vi plane as primary

2019-09-20 Thread roman . stratiienko
From: Roman Stratiienko 

DE2.0 blender does not take into the account alpha channel of vi layer.
Thus makes overlaying of this layer totally opaque.
Using vi layer as bottom solves this issue.

Tested on Android.

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +-
 2 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index dd2a1c851939..25183badc85f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -99,36 +99,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);
@@ -349,9 +319,6 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct 
drm_device *drm,
if (!layer)
return ERR_PTR(-ENOMEM);
 
-   if (index == 0)
-   type = DRM_PLANE_TYPE_PRIMARY;
-
/* possible crtcs are set later */
ret = drm_universal_plane_init(drm, >plane, 0,
   _ui_layer_funcs,
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 07c27e6a4b77..49c4074e164f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -116,6 +116,36 @@ static int sun8i_vi_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) & ~(format->hsub - 1),
@@ -445,6 +475,7 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct 
drm_device *drm,
   struct sun8i_mixer *mixer,
   int index)
 {
+   enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;
  

Re: [PATCH] drm/sun4i: Use vi plane as primary

2019-09-20 Thread Roman Stratiienko
Hello guys,

Actually, I beleive this is True solution, and current one is wrong.  Let
me explain why.

De2. 0 was designed to match Android hwcomposer hal requirements IMO.
You can easily agree with this conclusion by comparing Composer HAL and
De2. 0 hardware manuals.

I faced at least 4 issues when try to run Android using the mainline kernel
sun8i mixer implementation. Current one, missing pixel formats (my previous
patch), missing plane alpha and rotation properties. I plan to fix it and
also send appropriate solution to the upstream.

To achieve optimal UI performance Android requires at least 4 ui layers to
make screen composition. Current patch enables 4th plane usable.

As for using vi plane to display video. I assume that some of current users
may have regression in their software, but it could be easily fixed. For
example if vi layer isn't fullscreen and should be on top of the other
layers, it can actually be placed on the bottom and overlayed with pictures
with transparent rectangles in video region.
But I assume most of users such as browser etc. uses GPU for that.

And if you are watching fullscreen video, I can imagine only subtitles
layer and advertizing layers on top of the video layers.


чт, 19 сент. 2019 г., 21:15 Jernej Škrabec :

> Dne četrtek, 19. september 2019 ob 19:17:54 CEST je Maxime Ripard
> napisal(a):
> > Hi,
> >
> > On Thu, Sep 19, 2019 at 03:37:03PM +0300,
> roman.stratiie...@globallogic.com
> wrote:
> > > From: Roman Stratiienko 
> > >
> > > DE2.0 blender does not take into the account alpha channel of vi layer.
> > > Thus makes overlaying of this layer totally opaque.
> > > Using vi layer as bottom solves this issue.
>
> What issue? Overlays don't have to be "full screen", thus missing support
> for
> alpha blending doesn't make it less valuable. And VI planes are already
> placed
> at the bottom (zpos = 0).
>
> > >
> > > Tested on Android.
> > >
> > > Signed-off-by: Roman Stratiienko 
> >
> > It sounds like a workaround more than an actual fix.
> >
> > If the VI planes can't use the alpha, then we should just stop
> > reporting that format.
> >
> > Jernej, what do you think?
>
> Commit message is misleading. What this commit actually does is moving
> primary
> plane from first UI plane to bottom most plane, i.e. first VI plane.
> However, VI
> planes are scarce resource, almost all mixers have only one. I wouldn't
> set it
> as primary, because it's the only one which provide support for YUV
> formats.
> That could be used for example by video player for zero-copy rendering.
> Probably most apps wouldn't touch it if it was primary (that's usually
> reserved for window manager, if used).
>
> I left few formats with alpha channel exposed by VI planes, just because
> they
> don't have equivalent format without alpha. But I'm fine with removing
> them if
> you all agree on that.
>
> Best regards,
> Jernej
>
> >
> > Maxime
> >
> > > ---
> > >
> > >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ---
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +-
> > >  2 files changed, 35 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index
> dd2a1c851939..25183badc85f
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > @@ -99,36 +99,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;
> > > -
> > > -

[PATCH] drm/sun4i: Add missing pixel formats to the vi layer

2019-09-18 Thread roman . stratiienko
From: Roman Stratiienko 

According to Allwinner DE2.0 Specification REV 1.0, vi layer supports the
following pixel formats:  ABGR_, ARGB_, BGRA_, RGBA_

Signed-off-by: Roman Stratiienko 
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index bd0e6a52d1d8..07c27e6a4b77 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -404,17 +404,21 @@ static const struct drm_plane_funcs sun8i_vi_layer_funcs 
= {
 static const u32 sun8i_vi_layer_formats[] = {
DRM_FORMAT_ABGR1555,
DRM_FORMAT_ABGR,
+   DRM_FORMAT_ABGR,
DRM_FORMAT_ARGB1555,
DRM_FORMAT_ARGB,
+   DRM_FORMAT_ARGB,
DRM_FORMAT_BGR565,
DRM_FORMAT_BGR888,
DRM_FORMAT_BGRA5551,
DRM_FORMAT_BGRA,
+   DRM_FORMAT_BGRA,
DRM_FORMAT_BGRX,
DRM_FORMAT_RGB565,
DRM_FORMAT_RGB888,
DRM_FORMAT_RGBA,
DRM_FORMAT_RGBA5551,
+   DRM_FORMAT_RGBA,
DRM_FORMAT_RGBX,
DRM_FORMAT_XBGR,
DRM_FORMAT_XRGB,
-- 
2.17.1