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

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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

Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
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

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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

Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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

Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
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

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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 <roman.stratiienko@globallogic.com>
---
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

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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 <roman.stratiienko@globallogic.com>
---
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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[roman.stratiienko]

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

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 <roman.stratiienko@globallogic.com>
---
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

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

[Jernej Škrabec]

Hi!

Dne sreda, 01. januar 2020 ob 21:47:50 CET je 
roman.stratiienko@globallogic.com napisal(a):
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> 
> 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 <roman.stratiienko@globallogic.com>

This looks great now.

Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

What happened to other patches in the series? It would be nice to have a cover 
letter for such cases, where you can explain reasons for dropped patches.

Best regards,
Jernej

> ---
> v2:
> - Split commit in 2 parts
> - Add Fixes line to the commit message
> 
> v3:
> - Address review comments of v2 + removed 3 local varibles
> - Change 'Fixes' line
> 
> Since I've put more changes from my side, please review/sign again.
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
>  2 files changed, 28 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> format) return NULL;
>  }
> 
> +static void sun8i_mode_set(struct sunxi_engine *engine,
> +			   struct drm_display_mode *mode)
> +{
> +	u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
>crtc_vdisplay);
> +	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +	u32 bld_base = sun8i_blender_base(mixer);
> +	u32 val;
> +
> +	DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> +			 mode->crtc_hdisplay, mode->crtc_vdisplay);
> +	regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> +	regmap_write(mixer->engine.regs,
> +		     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +
> +	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> +	else
> +		val = 0;
> +
> +	regmap_update_bits(mixer->engine.regs,
> +			   SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> +			   SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> +			   val);
> +	DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> +			 val ? "on" : "off");
> +}
> +
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>  	DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
>  	.commit		= sun8i_mixer_commit,
>  	.layers_init	= sun8i_layers_init,
> +	.mode_set	= sun8i_mode_set,
>  };
> 
>  static struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
>  	outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> 
> -	if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> -		bool interlaced = false;
> -		u32 val;
> -
> -		DRM_DEBUG_DRIVER("Primary layer, updating global size 
W: %u H: %u\n",
> -				 dst_w, dst_h);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_GLOBAL_SIZE,
> -			     outsize);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), 
outsize);
> -
> -		if (state->crtc)
> -			interlaced = state->crtc->state-
>adjusted_mode.flags
> -				& DRM_MODE_FLAG_INTERLACE;
> -
> -		if (interlaced)
> -			val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> -		else
> -			val = 0;
> -
> -		regmap_update_bits(mixer->engine.regs,
> -				   
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> -				   
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> -				   val);
> -
> -		DRM_DEBUG_DRIVER("Switching display mixer interlaced 
mode %s\n",
> -				 interlaced ? "on" : "off");
> -	}
> -
>  	/* Set height and width */
>  	DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
>  			 state->src.x1 >> 16, state->src.y1 >> 16);

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

[Jernej Škrabec]

Hi!

Dne sreda, 01. januar 2020 ob 21:47:50 CET je 
roman.stratiienko@globallogic.com napisal(a):
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> 
> 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 <roman.stratiienko@globallogic.com>

This looks great now.

Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

What happened to other patches in the series? It would be nice to have a cover 
letter for such cases, where you can explain reasons for dropped patches.

Best regards,
Jernej

> ---
> v2:
> - Split commit in 2 parts
> - Add Fixes line to the commit message
> 
> v3:
> - Address review comments of v2 + removed 3 local varibles
> - Change 'Fixes' line
> 
> Since I've put more changes from my side, please review/sign again.
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
>  2 files changed, 28 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> format) return NULL;
>  }
> 
> +static void sun8i_mode_set(struct sunxi_engine *engine,
> +			   struct drm_display_mode *mode)
> +{
> +	u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
>crtc_vdisplay);
> +	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +	u32 bld_base = sun8i_blender_base(mixer);
> +	u32 val;
> +
> +	DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> +			 mode->crtc_hdisplay, mode->crtc_vdisplay);
> +	regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> +	regmap_write(mixer->engine.regs,
> +		     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +
> +	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> +	else
> +		val = 0;
> +
> +	regmap_update_bits(mixer->engine.regs,
> +			   SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> +			   SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> +			   val);
> +	DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> +			 val ? "on" : "off");
> +}
> +
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>  	DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
>  	.commit		= sun8i_mixer_commit,
>  	.layers_init	= sun8i_layers_init,
> +	.mode_set	= sun8i_mode_set,
>  };
> 
>  static struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
>  	outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> 
> -	if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> -		bool interlaced = false;
> -		u32 val;
> -
> -		DRM_DEBUG_DRIVER("Primary layer, updating global size 
W: %u H: %u\n",
> -				 dst_w, dst_h);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_GLOBAL_SIZE,
> -			     outsize);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), 
outsize);
> -
> -		if (state->crtc)
> -			interlaced = state->crtc->state-
>adjusted_mode.flags
> -				& DRM_MODE_FLAG_INTERLACE;
> -
> -		if (interlaced)
> -			val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> -		else
> -			val = 0;
> -
> -		regmap_update_bits(mixer->engine.regs,
> -				   
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> -				   
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> -				   val);
> -
> -		DRM_DEBUG_DRIVER("Switching display mixer interlaced 
mode %s\n",
> -				 interlaced ? "on" : "off");
> -	}
> -
>  	/* Set height and width */
>  	DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
>  			 state->src.x1 >> 16, state->src.y1 >> 16);





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jernej Škrabec]

Hi!

Dne sreda, 01. januar 2020 ob 21:47:50 CET je 
roman.stratiienko@globallogic.com napisal(a):
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> 
> 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 <roman.stratiienko@globallogic.com>

This looks great now.

Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

What happened to other patches in the series? It would be nice to have a cover 
letter for such cases, where you can explain reasons for dropped patches.

Best regards,
Jernej

> ---
> v2:
> - Split commit in 2 parts
> - Add Fixes line to the commit message
> 
> v3:
> - Address review comments of v2 + removed 3 local varibles
> - Change 'Fixes' line
> 
> Since I've put more changes from my side, please review/sign again.
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
>  2 files changed, 28 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> format) return NULL;
>  }
> 
> +static void sun8i_mode_set(struct sunxi_engine *engine,
> +			   struct drm_display_mode *mode)
> +{
> +	u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
>crtc_vdisplay);
> +	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +	u32 bld_base = sun8i_blender_base(mixer);
> +	u32 val;
> +
> +	DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> +			 mode->crtc_hdisplay, mode->crtc_vdisplay);
> +	regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> +	regmap_write(mixer->engine.regs,
> +		     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +
> +	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> +	else
> +		val = 0;
> +
> +	regmap_update_bits(mixer->engine.regs,
> +			   SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> +			   SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> +			   val);
> +	DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> +			 val ? "on" : "off");
> +}
> +
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>  	DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
>  	.commit		= sun8i_mixer_commit,
>  	.layers_init	= sun8i_layers_init,
> +	.mode_set	= sun8i_mode_set,
>  };
> 
>  static struct regmap_config sun8i_mixer_regmap_config = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
>  	outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> 
> -	if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> -		bool interlaced = false;
> -		u32 val;
> -
> -		DRM_DEBUG_DRIVER("Primary layer, updating global size 
W: %u H: %u\n",
> -				 dst_w, dst_h);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_GLOBAL_SIZE,
> -			     outsize);
> -		regmap_write(mixer->engine.regs,
> -			     SUN8I_MIXER_BLEND_OUTSIZE(bld_base), 
outsize);
> -
> -		if (state->crtc)
> -			interlaced = state->crtc->state-
>adjusted_mode.flags
> -				& DRM_MODE_FLAG_INTERLACE;
> -
> -		if (interlaced)
> -			val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> -		else
> -			val = 0;
> -
> -		regmap_update_bits(mixer->engine.regs,
> -				   
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> -				   
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> -				   val);
> -
> -		DRM_DEBUG_DRIVER("Switching display mixer interlaced 
mode %s\n",
> -				 interlaced ? "on" : "off");
> -	}
> -
>  	/* Set height and width */
>  	DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
>  			 state->src.x1 >> 16, state->src.y1 >> 16);




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

[Chen-Yu Tsai]

Hi Roman,

Your domain has DMARC setup with the "reject" policy.

This means emails from your domain may be subsequently rejected by people
using email forwarders (such as @kernel.org) to forward to Gmail.

I suggest using another email address to send patches, or ask your IT
people to drop the policy to "quarantine", which makes the email go to
the SPAM folder instead of outright rejecting them.

ChenYu

On Thu, Jan 2, 2020 at 3:43 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chen-Yu Tsai]

Hi Roman,

Your domain has DMARC setup with the "reject" policy.

This means emails from your domain may be subsequently rejected by people
using email forwarders (such as @kernel.org) to forward to Gmail.

I suggest using another email address to send patches, or ask your IT
people to drop the policy to "quarantine", which makes the email go to
the SPAM folder instead of outright rejecting them.

ChenYu

On Thu, Jan 2, 2020 at 3:43 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Chen-Yu Tsai]

Hi Roman,

Your domain has DMARC setup with the "reject" policy.

This means emails from your domain may be subsequently rejected by people
using email forwarders (such as @kernel.org) to forward to Gmail.

I suggest using another email address to send patches, or ask your IT
people to drop the policy to "quarantine", which makes the email go to
the SPAM folder instead of outright rejecting them.

ChenYu

On Thu, Jan 2, 2020 at 3:43 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
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.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

Hi,

On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
>
> 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 <roman.stratiienko@globallogic.com>

So it applies to all the 4 patches you've sent, but this lacks some
context.

There's a few questions that should be answered here:
  - Which situation is it fixing?
  - What tool / userspace stack is it fixing?
  - What happens with your fix? Do you set the plane at coordinates
    0,0 (meaning you'll crop the top-lef corner), do you center it? If
    the plane is smaller than the CTRC size, what is set on the edges?

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 962 bytes --]

Hi,

On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
>
> 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 <roman.stratiienko@globallogic.com>

So it applies to all the 4 patches you've sent, but this lacks some
context.

There's a few questions that should be answered here:
  - Which situation is it fixing?
  - What tool / userspace stack is it fixing?
  - What happens with your fix? Do you set the plane at coordinates
    0,0 (meaning you'll crop the top-lef corner), do you center it? If
    the plane is smaller than the CTRC size, what is set on the edges?

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 962 bytes --]

Hi,

On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
>
> 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 <roman.stratiienko@globallogic.com>

So it applies to all the 4 patches you've sent, but this lacks some
context.

There's a few questions that should be answered here:
  - Which situation is it fixing?
  - What tool / userspace stack is it fixing?
  - What happens with your fix? Do you set the plane at coordinates
    0,0 (meaning you'll crop the top-lef corner), do you center it? If
    the plane is smaller than the CTRC size, what is set on the edges?

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
>
> Hi,
>
> On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> 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

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
>
> Hi,
>
> On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
>
> Hi,
>
> On Wed, Jan 01, 2020 at 10:47:50PM +0200, roman.stratiienko@globallogic.com wrote:
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> 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

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 09:42 Jernej Škrabec <jernej.skrabec@siol.net>:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
>
>
>

Thanks and sorry for any mistakes in procedure, I'll try to follow the
rules in the future..
Some of commits requires more time to test/deliver than others.  So
splitting it into smaller chunks helps to deliver them earlier.

>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 09:42 Jernej Škrabec <jernej.skrabec@siol.net>:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
>
>
>

Thanks and sorry for any mistakes in procedure, I'll try to follow the
rules in the future..
Some of commits requires more time to test/deliver than others.  So
splitting it into smaller chunks helps to deliver them earlier.

>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Roman Stratiienko]

чт, 2 янв. 2020 г., 09:42 Jernej Škrabec <jernej.skrabec@siol.net>:
>
> Hi!
>
> Dne sreda, 01. januar 2020 ob 21:47:50 CET je
> roman.stratiienko@globallogic.com napisal(a):
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > 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 <roman.stratiienko@globallogic.com>
>
> This looks great now.
>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> What happened to other patches in the series? It would be nice to have a cover
> letter for such cases, where you can explain reasons for dropped patches.
>
>
>
>

Thanks and sorry for any mistakes in procedure, I'll try to follow the
rules in the future..
Some of commits requires more time to test/deliver than others.  So
splitting it into smaller chunks helps to deliver them earlier.

>
> Best regards,
> Jernej
>
> > ---
> > v2:
> > - Split commit in 2 parts
> > - Add Fixes line to the commit message
> >
> > v3:
> > - Address review comments of v2 + removed 3 local varibles
> > - Change 'Fixes' line
> >
> > Since I've put more changes from my side, please review/sign again.
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_mixer.c    | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 --------------------------
> >  2 files changed, 28 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..658cf442c121
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> > @@ -257,6 +257,33 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32
> > format) return NULL;
> >  }
> >
> > +static void sun8i_mode_set(struct sunxi_engine *engine,
> > +                        struct drm_display_mode *mode)
> > +{
> > +     u32 size = SUN8I_MIXER_SIZE(mode->crtc_hdisplay, mode-
> >crtc_vdisplay);
> > +     struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> > +     u32 bld_base = sun8i_blender_base(mixer);
> > +     u32 val;
> > +
> > +     DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
> > +                      mode->crtc_hdisplay, mode->crtc_vdisplay);
> > +     regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
> > +     regmap_write(mixer->engine.regs,
> > +                  SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> > +             val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > +     else
> > +             val = 0;
> > +
> > +     regmap_update_bits(mixer->engine.regs,
> > +                        SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > +                        SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > +                        val);
> > +     DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> > +                      val ? "on" : "off");
> > +}
> > +
> >  static void sun8i_mixer_commit(struct sunxi_engine *engine)
> >  {
> >       DRM_DEBUG_DRIVER("Committing changes\n");
> > @@ -310,6 +337,7 @@ static struct drm_plane **sun8i_layers_init(struct
> > drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = {
> >       .commit         = sun8i_mixer_commit,
> >       .layers_init    = sun8i_layers_init,
> > +     .mode_set       = sun8i_mode_set,
> >  };
> >
> >  static struct regmap_config sun8i_mixer_regmap_config = {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 4343ea9f8cf8..f01ac55191f1
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -120,36 +120,6 @@ static int sun8i_ui_layer_update_coord(struct
> > sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h);
> >       outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
> >
> > -     if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> > -             bool interlaced = false;
> > -             u32 val;
> > -
> > -             DRM_DEBUG_DRIVER("Primary layer, updating global size
> W: %u H: %u\n",
> > -                              dst_w, dst_h);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_GLOBAL_SIZE,
> > -                          outsize);
> > -             regmap_write(mixer->engine.regs,
> > -                          SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
> outsize);
> > -
> > -             if (state->crtc)
> > -                     interlaced = state->crtc->state-
> >adjusted_mode.flags
> > -                             & DRM_MODE_FLAG_INTERLACE;
> > -
> > -             if (interlaced)
> > -                     val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
> > -             else
> > -                     val = 0;
> > -
> > -             regmap_update_bits(mixer->engine.regs,
> > -
> SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> > -
> SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
> > -                                val);
> > -
> > -             DRM_DEBUG_DRIVER("Switching display mixer interlaced
> mode %s\n",
> > -                              interlaced ? "on" : "off");
> > -     }
> > -
> >       /* Set height and width */
> >       DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n",
> >                        state->src.x1 >> 16, state->src.y1 >> 16);
>
>
>
>
_______________________________________________
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.

[Jernej Škrabec]

Hi!

Dne četrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
> > Hi,
> > 
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200, 
roman.stratiienko@globallogic.com wrote:
> > > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> > > 
> > > 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 <roman.stratiienko@globallogic.com>
> > 
> > So it applies to all the 4 patches you've sent, but this lacks some
> > context.
> > 
> > There's a few questions that should be answered here:
> >   - Which situation is it fixing?
> 
> Setting primary plane size less than crtc breaks composition. Also
> shifting top left corner also breaks it.

True, HW doesn't have notion of primary plane. It's just one plane which is 
marked as primary, but otherwise it has same capabilities as others, like x,y 
coordinates, size, etc.

> 
> >   - What tool / userspace stack is it fixing?
> 
> I am using Android userspace and drm_hwcomposer HAL.
> 
> @Jernej, you've said that you observed similar issue. Could you share
> what userspace have you used?

I observed it with DE1, but it has exactly the same issue. I noticed this 
problem on Kodi (gbm version). Kodi first searches for plane capable of 
displaying NV12 format (for video) and after that a plane which is capable of 
displaying RGB888 format (for GUI). In DE1 case, first plane is primary and 
also capable of displaying NV12 format. So when video is displayed which 
doesn't cover whole screen, display output is corrupted. However, with such 
fix, video playback is correct. Luc Verhaegen make equivalent fix for DE1, where 
he also claims primary plane doesn't have to be same size as CRTC output:
https://github.com/libv/fosdem-video-linux/commit/
ae3215d37ca2a55642bcae6c83c3612e26275711

> 
> >   - What happens with your fix? Do you set the plane at coordinates
> >   
> >     0,0 (meaning you'll crop the top-lef corner), do you center it? If
> >     the plane is smaller than the CTRC size, what is set on the edges?
> 
> You can put primary plane to any part of the screen and make it as
> small as 8x8 (according to the datasheet) . Background would be filled
> with black color, that is default, but it also could be overridden by
> setting corresponding registers.

Correct, same logic as for overlay planes applies.

Best regards,
Jernej

> 
> > Thanks!
> > Maxime

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

[Jernej Škrabec]

Hi!

Dne četrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
> > Hi,
> > 
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200, 
roman.stratiienko@globallogic.com wrote:
> > > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> > > 
> > > 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 <roman.stratiienko@globallogic.com>
> > 
> > So it applies to all the 4 patches you've sent, but this lacks some
> > context.
> > 
> > There's a few questions that should be answered here:
> >   - Which situation is it fixing?
> 
> Setting primary plane size less than crtc breaks composition. Also
> shifting top left corner also breaks it.

True, HW doesn't have notion of primary plane. It's just one plane which is 
marked as primary, but otherwise it has same capabilities as others, like x,y 
coordinates, size, etc.

> 
> >   - What tool / userspace stack is it fixing?
> 
> I am using Android userspace and drm_hwcomposer HAL.
> 
> @Jernej, you've said that you observed similar issue. Could you share
> what userspace have you used?

I observed it with DE1, but it has exactly the same issue. I noticed this 
problem on Kodi (gbm version). Kodi first searches for plane capable of 
displaying NV12 format (for video) and after that a plane which is capable of 
displaying RGB888 format (for GUI). In DE1 case, first plane is primary and 
also capable of displaying NV12 format. So when video is displayed which 
doesn't cover whole screen, display output is corrupted. However, with such 
fix, video playback is correct. Luc Verhaegen make equivalent fix for DE1, where 
he also claims primary plane doesn't have to be same size as CRTC output:
https://github.com/libv/fosdem-video-linux/commit/
ae3215d37ca2a55642bcae6c83c3612e26275711

> 
> >   - What happens with your fix? Do you set the plane at coordinates
> >   
> >     0,0 (meaning you'll crop the top-lef corner), do you center it? If
> >     the plane is smaller than the CTRC size, what is set on the edges?
> 
> You can put primary plane to any part of the screen and make it as
> small as 8x8 (according to the datasheet) . Background would be filled
> with black color, that is default, but it also could be overridden by
> setting corresponding registers.

Correct, same logic as for overlay planes applies.

Best regards,
Jernej

> 
> > Thanks!
> > Maxime





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jernej Škrabec]

Hi!

Dne četrtek, 02. januar 2020 ob 17:32:07 CET je Roman Stratiienko napisal(a):
> чт, 2 янв. 2020 г., 12:08 Maxime Ripard <mripard@kernel.org>:
> > Hi,
> > 
> > On Wed, Jan 01, 2020 at 10:47:50PM +0200, 
roman.stratiienko@globallogic.com wrote:
> > > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> > > 
> > > 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 <roman.stratiienko@globallogic.com>
> > 
> > So it applies to all the 4 patches you've sent, but this lacks some
> > context.
> > 
> > There's a few questions that should be answered here:
> >   - Which situation is it fixing?
> 
> Setting primary plane size less than crtc breaks composition. Also
> shifting top left corner also breaks it.

True, HW doesn't have notion of primary plane. It's just one plane which is 
marked as primary, but otherwise it has same capabilities as others, like x,y 
coordinates, size, etc.

> 
> >   - What tool / userspace stack is it fixing?
> 
> I am using Android userspace and drm_hwcomposer HAL.
> 
> @Jernej, you've said that you observed similar issue. Could you share
> what userspace have you used?

I observed it with DE1, but it has exactly the same issue. I noticed this 
problem on Kodi (gbm version). Kodi first searches for plane capable of 
displaying NV12 format (for video) and after that a plane which is capable of 
displaying RGB888 format (for GUI). In DE1 case, first plane is primary and 
also capable of displaying NV12 format. So when video is displayed which 
doesn't cover whole screen, display output is corrupted. However, with such 
fix, video playback is correct. Luc Verhaegen make equivalent fix for DE1, where 
he also claims primary plane doesn't have to be same size as CRTC output:
https://github.com/libv/fosdem-video-linux/commit/
ae3215d37ca2a55642bcae6c83c3612e26275711

> 
> >   - What happens with your fix? Do you set the plane at coordinates
> >   
> >     0,0 (meaning you'll crop the top-lef corner), do you center it? If
> >     the plane is smaller than the CTRC size, what is set on the edges?
> 
> You can put primary plane to any part of the screen and make it as
> small as 8x8 (according to the datasheet) . Background would be filled
> with black color, that is default, but it also could be overridden by
> setting corresponding registers.

Correct, same logic as for overlay planes applies.

Best regards,
Jernej

> 
> > Thanks!
> > Maxime




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