* [PATCH] drm/komeda: Adds zorder support
@ 2019-05-20 3:33 Lowry Li (Arm Technology China)
2019-05-20 6:05 ` james qian wang (Arm Technology China)
2019-10-08 15:00 ` [PATCH] " Daniel Vetter
0 siblings, 2 replies; 4+ messages in thread
From: Lowry Li (Arm Technology China) @ 2019-05-20 3:33 UTC (permalink / raw)
To: Liviu Dudau, james qian wang (Arm Technology China),
maarten.lankhorst, seanpaul, airlied, Brian Starkey
Cc: Julien Yin (Arm Technology China),
Jonathan Chai (Arm Technology China),
Ayan Halder, dri-devel, linux-kernel, nd
- Creates the zpos property.
- Implement komeda_crtc_normalize_zpos to replace
drm_atomic_normalize_zpos, reasons as the following:
1. The drm_atomic_normalize_zpos allows to configure same zpos for
different planes, but komeda doesn't support such configuration.
2. For further slave pipline case, Komeda need to calculate the
max_slave_zorder, we will merge such calculation into
komed_crtc_normalize_zpos to save a separated plane_state loop.
3. For feature none-scaling layer_split, which a plane_state will be
assigned to two individual layers(left/right), which requires two
normalize_zpos for this plane, plane_st->normalize_zpos will be used
by left layer, normalize_zpos + 1 for right_layer.
This patch series depends on:
- https://patchwork.freedesktop.org/series/58710/
- https://patchwork.freedesktop.org/series/59000/
- https://patchwork.freedesktop.org/series/59002/
- https://patchwork.freedesktop.org/series/59747/
- https://patchwork.freedesktop.org/series/59915/
- https://patchwork.freedesktop.org/series/60083/
- https://patchwork.freedesktop.org/series/60698/
Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 90 ++++++++++++++++++++++-
drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 +
drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-
3 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 306ea06..0ec7665 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
.atomic_commit_tail = komeda_kms_commit_tail,
};
+static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
+ struct list_head *zorder_list)
+{
+ struct komeda_plane_state *new = to_kplane_st(plane_st);
+ struct komeda_plane_state *node, *last;
+
+ last = list_empty(zorder_list) ?
+ NULL : list_last_entry(zorder_list, typeof(*last), zlist_node);
+
+ /* Considering the list sequence is zpos increasing, so if list is empty
+ * or the zpos of new node bigger than the last node in list, no need
+ * loop and just insert the new one to the tail of the list.
+ */
+ if (!last || (new->base.zpos > last->base.zpos)) {
+ list_add_tail(&new->zlist_node, zorder_list);
+ return 0;
+ }
+
+ /* Build the list by zpos increasing */
+ list_for_each_entry(node, zorder_list, zlist_node) {
+ if (new->base.zpos < node->base.zpos) {
+ list_add_tail(&new->zlist_node, &node->zlist_node);
+ break;
+ } else if (node->base.zpos == new->base.zpos) {
+ struct drm_plane *a = node->base.plane;
+ struct drm_plane *b = new->base.plane;
+
+ /* Komeda doesn't support setting a same zpos for
+ * different planes.
+ */
+ DRM_DEBUG_ATOMIC("PLANE: %s and PLANE: %s are configured same zpos: %d.\n",
+ a->name, b->name, node->base.zpos);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
+ struct drm_crtc_state *crtc_st)
+{
+ struct drm_atomic_state *state = crtc_st->state;
+ struct komeda_plane_state *kplane_st;
+ struct drm_plane_state *plane_st;
+ struct drm_framebuffer *fb;
+ struct drm_plane *plane;
+ struct list_head zorder_list;
+ int order = 0, err;
+
+ DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+ crtc->base.id, crtc->name);
+
+ INIT_LIST_HEAD(&zorder_list);
+
+ /* This loop also added all effected planes into the new state */
+ drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
+ plane_st = drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_st))
+ return PTR_ERR(plane_st);
+
+ /* Build a list by zpos increasing */
+ err = komeda_plane_state_list_add(plane_st, &zorder_list);
+ if (err)
+ return err;
+ }
+
+ list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
+ plane_st = &kplane_st->base;
+ fb = plane_st->fb;
+ plane = plane_st->plane;
+
+ plane_st->normalized_zpos = order++;
+
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
+ plane->base.id, plane->name,
+ plane_st->zpos, plane_st->normalized_zpos);
+ }
+
+ crtc_st->zpos_changed = true;
+
+ return 0;
+}
+
static int komeda_kms_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
@@ -111,7 +195,7 @@ static int komeda_kms_check(struct drm_device *dev,
if (err)
return err;
- /* komeda need to re-calculate resource assumption in every commit
+ /* Komeda need to re-calculate resource assumption in every commit
* so need to add all affected_planes (even unchanged) to
* drm_atomic_state.
*/
@@ -119,6 +203,10 @@ static int komeda_kms_check(struct drm_device *dev,
err = drm_atomic_add_affected_planes(state, crtc);
if (err)
return err;
+
+ err = komeda_crtc_normalize_zpos(crtc, new_crtc_st);
+ if (err)
+ return err;
}
err = drm_atomic_helper_check_planes(dev, state);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 178bee6..d1cef46 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -7,6 +7,7 @@
#ifndef _KOMEDA_KMS_H_
#define _KOMEDA_KMS_H_
+#include <linux/list.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_crtc_helper.h>
@@ -46,6 +47,8 @@ struct komeda_plane {
struct komeda_plane_state {
/** @base: &drm_plane_state */
struct drm_plane_state base;
+ /** @zlist_node: zorder list node */
+ struct list_head zlist_node;
/* @img_enhancement: on/off image enhancement */
u8 img_enhancement : 1;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index bcf30a7..aad7663 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -21,7 +21,7 @@
memset(dflow, 0, sizeof(*dflow));
- dflow->blending_zorder = st->zpos;
+ dflow->blending_zorder = st->normalized_zpos;
/* if format doesn't have alpha, fix blend mode to PIXEL_NONE */
dflow->pixel_blend_mode = fb->format->has_alpha ?
@@ -343,6 +343,10 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
if (err)
goto cleanup;
+ err = drm_plane_create_zpos_property(plane, layer->base.id, 0, 8);
+ if (err)
+ goto cleanup;
+
return 0;
cleanup:
komeda_plane_destroy(plane);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: drm/komeda: Adds zorder support
2019-05-20 3:33 [PATCH] drm/komeda: Adds zorder support Lowry Li (Arm Technology China)
@ 2019-05-20 6:05 ` james qian wang (Arm Technology China)
2019-10-08 15:00 ` [PATCH] " Daniel Vetter
1 sibling, 0 replies; 4+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-05-20 6:05 UTC (permalink / raw)
To: Lowry Li (Arm Technology China)
Cc: Liviu Dudau, maarten.lankhorst, seanpaul, airlied, Brian Starkey,
Ayan Halder, Jonathan Chai (Arm Technology China),
linux-kernel, dri-devel, Julien Yin (Arm Technology China),
nd
On Mon, May 20, 2019 at 03:33:19AM +0000, Lowry Li (Arm Technology China) wrote:
> - Creates the zpos property.
> - Implement komeda_crtc_normalize_zpos to replace
> drm_atomic_normalize_zpos, reasons as the following:
>
> 1. The drm_atomic_normalize_zpos allows to configure same zpos for
> different planes, but komeda doesn't support such configuration.
> 2. For further slave pipline case, Komeda need to calculate the
> max_slave_zorder, we will merge such calculation into
> komed_crtc_normalize_zpos to save a separated plane_state loop.
> 3. For feature none-scaling layer_split, which a plane_state will be
> assigned to two individual layers(left/right), which requires two
> normalize_zpos for this plane, plane_st->normalize_zpos will be used
> by left layer, normalize_zpos + 1 for right_layer.
>
> This patch series depends on:
> - https://patchwork.freedesktop.org/series/58710/
> - https://patchwork.freedesktop.org/series/59000/
> - https://patchwork.freedesktop.org/series/59002/
> - https://patchwork.freedesktop.org/series/59747/
> - https://patchwork.freedesktop.org/series/59915/
> - https://patchwork.freedesktop.org/series/60083/
> - https://patchwork.freedesktop.org/series/60698/
>
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 90 ++++++++++++++++++++++-
> drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 +
> drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-
> 3 files changed, 97 insertions(+), 2 deletions(-)
>
> --
> 1.9.1
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 306ea06..0ec7665 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
> .atomic_commit_tail = komeda_kms_commit_tail,
> };
>
> +static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
> + struct list_head *zorder_list)
> +{
> + struct komeda_plane_state *new = to_kplane_st(plane_st);
> + struct komeda_plane_state *node, *last;
> +
> + last = list_empty(zorder_list) ?
> + NULL : list_last_entry(zorder_list, typeof(*last), zlist_node);
> +
> + /* Considering the list sequence is zpos increasing, so if list is empty
> + * or the zpos of new node bigger than the last node in list, no need
> + * loop and just insert the new one to the tail of the list.
> + */
> + if (!last || (new->base.zpos > last->base.zpos)) {
> + list_add_tail(&new->zlist_node, zorder_list);
> + return 0;
> + }
> +
> + /* Build the list by zpos increasing */
> + list_for_each_entry(node, zorder_list, zlist_node) {
> + if (new->base.zpos < node->base.zpos) {
> + list_add_tail(&new->zlist_node, &node->zlist_node);
> + break;
> + } else if (node->base.zpos == new->base.zpos) {
> + struct drm_plane *a = node->base.plane;
> + struct drm_plane *b = new->base.plane;
> +
> + /* Komeda doesn't support setting a same zpos for
> + * different planes.
> + */
> + DRM_DEBUG_ATOMIC("PLANE: %s and PLANE: %s are configured same zpos: %d.\n",
> + a->name, b->name, node->base.zpos);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_st)
> +{
> + struct drm_atomic_state *state = crtc_st->state;
> + struct komeda_plane_state *kplane_st;
> + struct drm_plane_state *plane_st;
> + struct drm_framebuffer *fb;
> + struct drm_plane *plane;
> + struct list_head zorder_list;
> + int order = 0, err;
> +
> + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> + crtc->base.id, crtc->name);
> +
> + INIT_LIST_HEAD(&zorder_list);
> +
> + /* This loop also added all effected planes into the new state */
> + drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
> + plane_st = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_st))
> + return PTR_ERR(plane_st);
> +
> + /* Build a list by zpos increasing */
> + err = komeda_plane_state_list_add(plane_st, &zorder_list);
> + if (err)
> + return err;
> + }
> +
> + list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
> + plane_st = &kplane_st->base;
> + fb = plane_st->fb;
> + plane = plane_st->plane;
> +
> + plane_st->normalized_zpos = order++;
> +
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
> + plane->base.id, plane->name,
> + plane_st->zpos, plane_st->normalized_zpos);
> + }
> +
> + crtc_st->zpos_changed = true;
> +
> + return 0;
> +}
> +
> static int komeda_kms_check(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> @@ -111,7 +195,7 @@ static int komeda_kms_check(struct drm_device *dev,
> if (err)
> return err;
>
> - /* komeda need to re-calculate resource assumption in every commit
> + /* Komeda need to re-calculate resource assumption in every commit
> * so need to add all affected_planes (even unchanged) to
> * drm_atomic_state.
> */
> @@ -119,6 +203,10 @@ static int komeda_kms_check(struct drm_device *dev,
> err = drm_atomic_add_affected_planes(state, crtc);
> if (err)
> return err;
> +
> + err = komeda_crtc_normalize_zpos(crtc, new_crtc_st);
> + if (err)
> + return err;
> }
>
> err = drm_atomic_helper_check_planes(dev, state);
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 178bee6..d1cef46 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -7,6 +7,7 @@
> #ifndef _KOMEDA_KMS_H_
> #define _KOMEDA_KMS_H_
>
> +#include <linux/list.h>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
> @@ -46,6 +47,8 @@ struct komeda_plane {
> struct komeda_plane_state {
> /** @base: &drm_plane_state */
> struct drm_plane_state base;
> + /** @zlist_node: zorder list node */
> + struct list_head zlist_node;
>
> /* @img_enhancement: on/off image enhancement */
> u8 img_enhancement : 1;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index bcf30a7..aad7663 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -21,7 +21,7 @@
>
> memset(dflow, 0, sizeof(*dflow));
>
> - dflow->blending_zorder = st->zpos;
> + dflow->blending_zorder = st->normalized_zpos;
>
> /* if format doesn't have alpha, fix blend mode to PIXEL_NONE */
> dflow->pixel_blend_mode = fb->format->has_alpha ?
> @@ -343,6 +343,10 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
> if (err)
> goto cleanup;
>
> + err = drm_plane_create_zpos_property(plane, layer->base.id, 0, 8);
> + if (err)
> + goto cleanup;
> +
> return 0;
> cleanup:
> komeda_plane_destroy(plane);
Looks good to me.
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.wang@arm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/komeda: Adds zorder support
2019-05-20 3:33 [PATCH] drm/komeda: Adds zorder support Lowry Li (Arm Technology China)
2019-05-20 6:05 ` james qian wang (Arm Technology China)
@ 2019-10-08 15:00 ` Daniel Vetter
2019-10-09 5:15 ` james qian wang (Arm Technology China)
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2019-10-08 15:00 UTC (permalink / raw)
To: Lowry Li (Arm Technology China)
Cc: Liviu Dudau, james qian wang (Arm Technology China),
maarten.lankhorst, seanpaul, airlied, Brian Starkey, Ayan Halder,
Jonathan Chai (Arm Technology China),
linux-kernel, dri-devel, Julien Yin (Arm Technology China),
nd
On Mon, May 20, 2019 at 5:33 AM Lowry Li (Arm Technology China)
<Lowry.Li@arm.com> wrote:
>
> - Creates the zpos property.
> - Implement komeda_crtc_normalize_zpos to replace
> drm_atomic_normalize_zpos, reasons as the following:
>
> 1. The drm_atomic_normalize_zpos allows to configure same zpos for
> different planes, but komeda doesn't support such configuration.
Just stumbled over your custom normalized_zpos calculation, and it
looks very fishy. You seem to reinvent the normalized zpos
computation, which has the entire job of resolving duplicated zpos
values (which can happen with legacy kms). So the above is definitely
wrong.
Can you pls do a patch to remove your own code, and replace it with
helper usage? Or at least explain why exactly you can't use the
standard normalized zpos stuff and need your own (since it really
looks like just reinventing the same thing).
-Daniel
> 2. For further slave pipline case, Komeda need to calculate the
> max_slave_zorder, we will merge such calculation into
> komed_crtc_normalize_zpos to save a separated plane_state loop.
> 3. For feature none-scaling layer_split, which a plane_state will be
> assigned to two individual layers(left/right), which requires two
> normalize_zpos for this plane, plane_st->normalize_zpos will be used
> by left layer, normalize_zpos + 1 for right_layer.
>
> This patch series depends on:
> - https://patchwork.freedesktop.org/series/58710/
> - https://patchwork.freedesktop.org/series/59000/
> - https://patchwork.freedesktop.org/series/59002/
> - https://patchwork.freedesktop.org/series/59747/
> - https://patchwork.freedesktop.org/series/59915/
> - https://patchwork.freedesktop.org/series/60083/
> - https://patchwork.freedesktop.org/series/60698/
>
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 90 ++++++++++++++++++++++-
> drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 +
> drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-
> 3 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 306ea06..0ec7665 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
> .atomic_commit_tail = komeda_kms_commit_tail,
> };
>
> +static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
> + struct list_head *zorder_list)
> +{
> + struct komeda_plane_state *new = to_kplane_st(plane_st);
> + struct komeda_plane_state *node, *last;
> +
> + last = list_empty(zorder_list) ?
> + NULL : list_last_entry(zorder_list, typeof(*last), zlist_node);
> +
> + /* Considering the list sequence is zpos increasing, so if list is empty
> + * or the zpos of new node bigger than the last node in list, no need
> + * loop and just insert the new one to the tail of the list.
> + */
> + if (!last || (new->base.zpos > last->base.zpos)) {
> + list_add_tail(&new->zlist_node, zorder_list);
> + return 0;
> + }
> +
> + /* Build the list by zpos increasing */
> + list_for_each_entry(node, zorder_list, zlist_node) {
> + if (new->base.zpos < node->base.zpos) {
> + list_add_tail(&new->zlist_node, &node->zlist_node);
> + break;
> + } else if (node->base.zpos == new->base.zpos) {
> + struct drm_plane *a = node->base.plane;
> + struct drm_plane *b = new->base.plane;
> +
> + /* Komeda doesn't support setting a same zpos for
> + * different planes.
> + */
> + DRM_DEBUG_ATOMIC("PLANE: %s and PLANE: %s are configured same zpos: %d.\n",
> + a->name, b->name, node->base.zpos);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_st)
> +{
> + struct drm_atomic_state *state = crtc_st->state;
> + struct komeda_plane_state *kplane_st;
> + struct drm_plane_state *plane_st;
> + struct drm_framebuffer *fb;
> + struct drm_plane *plane;
> + struct list_head zorder_list;
> + int order = 0, err;
> +
> + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> + crtc->base.id, crtc->name);
> +
> + INIT_LIST_HEAD(&zorder_list);
> +
> + /* This loop also added all effected planes into the new state */
> + drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
> + plane_st = drm_atomic_get_plane_state(state, plane);
> + if (IS_ERR(plane_st))
> + return PTR_ERR(plane_st);
> +
> + /* Build a list by zpos increasing */
> + err = komeda_plane_state_list_add(plane_st, &zorder_list);
> + if (err)
> + return err;
> + }
> +
> + list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
> + plane_st = &kplane_st->base;
> + fb = plane_st->fb;
> + plane = plane_st->plane;
> +
> + plane_st->normalized_zpos = order++;
> +
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
> + plane->base.id, plane->name,
> + plane_st->zpos, plane_st->normalized_zpos);
> + }
> +
> + crtc_st->zpos_changed = true;
> +
> + return 0;
> +}
> +
> static int komeda_kms_check(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> @@ -111,7 +195,7 @@ static int komeda_kms_check(struct drm_device *dev,
> if (err)
> return err;
>
> - /* komeda need to re-calculate resource assumption in every commit
> + /* Komeda need to re-calculate resource assumption in every commit
> * so need to add all affected_planes (even unchanged) to
> * drm_atomic_state.
> */
> @@ -119,6 +203,10 @@ static int komeda_kms_check(struct drm_device *dev,
> err = drm_atomic_add_affected_planes(state, crtc);
> if (err)
> return err;
> +
> + err = komeda_crtc_normalize_zpos(crtc, new_crtc_st);
> + if (err)
> + return err;
> }
>
> err = drm_atomic_helper_check_planes(dev, state);
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> index 178bee6..d1cef46 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> @@ -7,6 +7,7 @@
> #ifndef _KOMEDA_KMS_H_
> #define _KOMEDA_KMS_H_
>
> +#include <linux/list.h>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_crtc_helper.h>
> @@ -46,6 +47,8 @@ struct komeda_plane {
> struct komeda_plane_state {
> /** @base: &drm_plane_state */
> struct drm_plane_state base;
> + /** @zlist_node: zorder list node */
> + struct list_head zlist_node;
>
> /* @img_enhancement: on/off image enhancement */
> u8 img_enhancement : 1;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index bcf30a7..aad7663 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -21,7 +21,7 @@
>
> memset(dflow, 0, sizeof(*dflow));
>
> - dflow->blending_zorder = st->zpos;
> + dflow->blending_zorder = st->normalized_zpos;
>
> /* if format doesn't have alpha, fix blend mode to PIXEL_NONE */
> dflow->pixel_blend_mode = fb->format->has_alpha ?
> @@ -343,6 +343,10 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
> if (err)
> goto cleanup;
>
> + err = drm_plane_create_zpos_property(plane, layer->base.id, 0, 8);
> + if (err)
> + goto cleanup;
> +
> return 0;
> cleanup:
> komeda_plane_destroy(plane);
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/komeda: Adds zorder support
2019-10-08 15:00 ` [PATCH] " Daniel Vetter
@ 2019-10-09 5:15 ` james qian wang (Arm Technology China)
0 siblings, 0 replies; 4+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-10-09 5:15 UTC (permalink / raw)
To: Daniel Vetter
Cc: Lowry Li (Arm Technology China),
Liviu Dudau, maarten.lankhorst, seanpaul, airlied, Brian Starkey,
Ayan Halder, Jonathan Chai (Arm Technology China),
linux-kernel, dri-devel, Julien Yin (Arm Technology China),
nd
On Tue, Oct 08, 2019 at 05:00:46PM +0200, Daniel Vetter wrote:
> On Mon, May 20, 2019 at 5:33 AM Lowry Li (Arm Technology China)
> <Lowry.Li@arm.com> wrote:
> >
> > - Creates the zpos property.
> > - Implement komeda_crtc_normalize_zpos to replace
> > drm_atomic_normalize_zpos, reasons as the following:
> >
> > 1. The drm_atomic_normalize_zpos allows to configure same zpos for
> > different planes, but komeda doesn't support such configuration.
>
> Just stumbled over your custom normalized_zpos calculation, and it
> looks very fishy. You seem to reinvent the normalized zpos
> computation, which has the entire job of resolving duplicated zpos
> values (which can happen with legacy kms). So the above is definitely
> wrong.
>
> Can you pls do a patch to remove your own code, and replace it with
> helper usage? Or at least explain why exactly you can't use the
> standard normalized zpos stuff and need your own (since it really
> looks like just reinventing the same thing).
> -Daniel
>
> > 2. For further slave pipline case, Komeda need to calculate the
> > max_slave_zorder, we will merge such calculation into
> > komed_crtc_normalize_zpos to save a separated plane_state loop.
> > 3. For feature none-scaling layer_split, which a plane_state will be
> > assigned to two individual layers(left/right), which requires two
> > normalize_zpos for this plane, plane_st->normalize_zpos will be used
> > by left layer, normalize_zpos + 1 for right_layer.
> >
Yes, the komeda/drm zpos_normalize are simlilar,
First requirement is here the "layer split" support.
Komeda HW has a input size limitation for YUV format, compare with
RGB, which only has the half capablities, like
- Layer support 4K RGB input size.
- YUV only can support 2K.
To hide this limitation, we introdue Layer split, which splits
one drm_plane data flows to two halves (left/right), and handle them by
two individual HW layers, and of course, two layers need two different
normalize_zpos. but drm_atomic_normalize_zpos() only assign one value
to this plane, but we need two values for layer_split.
Secondary requirement:
we also need the ordered plane list that built by normalize_zpos(),
but current drm version doesn't return it.
For more komeda things:
https://www.kernel.org/doc/html/latest/gpu/komeda-kms.html?highlight=komeda
LAST.
Yes, the komeda and drm version zpos normalize are similar,
we can not use the standard version only because some tiny but
special requirements.
I saw the upstream is working for the zorder refinement, maybe we
can add komeda's requirements into the drm_atomic_normalize_zpos()
and drop komeda version together with this refinement.
Best Regards
James
> > This patch series depends on:
> > - https://patchwork.freedesktop.org/series/58710/
> > - https://patchwork.freedesktop.org/series/59000/
> > - https://patchwork.freedesktop.org/series/59002/
> > - https://patchwork.freedesktop.org/series/59747/
> > - https://patchwork.freedesktop.org/series/59915/
> > - https://patchwork.freedesktop.org/series/60083/
> > - https://patchwork.freedesktop.org/series/60698/
> >
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 90 ++++++++++++++++++++++-
> > drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 +
> > drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-
> > 3 files changed, 97 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index 306ea06..0ec7665 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state)
> > .atomic_commit_tail = komeda_kms_commit_tail,
> > };
> >
> > +static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
> > + struct list_head *zorder_list)
> > +{
> > + struct komeda_plane_state *new = to_kplane_st(plane_st);
> > + struct komeda_plane_state *node, *last;
> > +
> > + last = list_empty(zorder_list) ?
> > + NULL : list_last_entry(zorder_list, typeof(*last), zlist_node);
> > +
> > + /* Considering the list sequence is zpos increasing, so if list is empty
> > + * or the zpos of new node bigger than the last node in list, no need
> > + * loop and just insert the new one to the tail of the list.
> > + */
> > + if (!last || (new->base.zpos > last->base.zpos)) {
> > + list_add_tail(&new->zlist_node, zorder_list);
> > + return 0;
> > + }
> > +
> > + /* Build the list by zpos increasing */
> > + list_for_each_entry(node, zorder_list, zlist_node) {
> > + if (new->base.zpos < node->base.zpos) {
> > + list_add_tail(&new->zlist_node, &node->zlist_node);
> > + break;
> > + } else if (node->base.zpos == new->base.zpos) {
> > + struct drm_plane *a = node->base.plane;
> > + struct drm_plane *b = new->base.plane;
> > +
> > + /* Komeda doesn't support setting a same zpos for
> > + * different planes.
> > + */
> > + DRM_DEBUG_ATOMIC("PLANE: %s and PLANE: %s are configured same zpos: %d.\n",
> > + a->name, b->name, node->base.zpos);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> > + struct drm_crtc_state *crtc_st)
> > +{
> > + struct drm_atomic_state *state = crtc_st->state;
> > + struct komeda_plane_state *kplane_st;
> > + struct drm_plane_state *plane_st;
> > + struct drm_framebuffer *fb;
> > + struct drm_plane *plane;
> > + struct list_head zorder_list;
> > + int order = 0, err;
> > +
> > + DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> > + crtc->base.id, crtc->name);
> > +
> > + INIT_LIST_HEAD(&zorder_list);
> > +
> > + /* This loop also added all effected planes into the new state */
> > + drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
> > + plane_st = drm_atomic_get_plane_state(state, plane);
> > + if (IS_ERR(plane_st))
> > + return PTR_ERR(plane_st);
> > +
> > + /* Build a list by zpos increasing */
> > + err = komeda_plane_state_list_add(plane_st, &zorder_list);
> > + if (err)
> > + return err;
> > + }
> > +
> > + list_for_each_entry(kplane_st, &zorder_list, zlist_node) {
> > + plane_st = &kplane_st->base;
> > + fb = plane_st->fb;
> > + plane = plane_st->plane;
> > +
> > + plane_st->normalized_zpos = order++;
> > +
> > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] zpos:%d, normalized zpos: %d\n",
> > + plane->base.id, plane->name,
> > + plane_st->zpos, plane_st->normalized_zpos);
> > + }
> > +
> > + crtc_st->zpos_changed = true;
> > +
> > + return 0;
> > +}
> > +
> > static int komeda_kms_check(struct drm_device *dev,
> > struct drm_atomic_state *state)
> > {
> > @@ -111,7 +195,7 @@ static int komeda_kms_check(struct drm_device *dev,
> > if (err)
> > return err;
> >
> > - /* komeda need to re-calculate resource assumption in every commit
> > + /* Komeda need to re-calculate resource assumption in every commit
> > * so need to add all affected_planes (even unchanged) to
> > * drm_atomic_state.
> > */
> > @@ -119,6 +203,10 @@ static int komeda_kms_check(struct drm_device *dev,
> > err = drm_atomic_add_affected_planes(state, crtc);
> > if (err)
> > return err;
> > +
> > + err = komeda_crtc_normalize_zpos(crtc, new_crtc_st);
> > + if (err)
> > + return err;
> > }
> >
> > err = drm_atomic_helper_check_planes(dev, state);
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > index 178bee6..d1cef46 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
> > @@ -7,6 +7,7 @@
> > #ifndef _KOMEDA_KMS_H_
> > #define _KOMEDA_KMS_H_
> >
> > +#include <linux/list.h>
> > #include <drm/drm_atomic.h>
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_crtc_helper.h>
> > @@ -46,6 +47,8 @@ struct komeda_plane {
> > struct komeda_plane_state {
> > /** @base: &drm_plane_state */
> > struct drm_plane_state base;
> > + /** @zlist_node: zorder list node */
> > + struct list_head zlist_node;
> >
> > /* @img_enhancement: on/off image enhancement */
> > u8 img_enhancement : 1;
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > index bcf30a7..aad7663 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> > @@ -21,7 +21,7 @@
> >
> > memset(dflow, 0, sizeof(*dflow));
> >
> > - dflow->blending_zorder = st->zpos;
> > + dflow->blending_zorder = st->normalized_zpos;
> >
> > /* if format doesn't have alpha, fix blend mode to PIXEL_NONE */
> > dflow->pixel_blend_mode = fb->format->has_alpha ?
> > @@ -343,6 +343,10 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
> > if (err)
> > goto cleanup;
> >
> > + err = drm_plane_create_zpos_property(plane, layer->base.id, 0, 8);
> > + if (err)
> > + goto cleanup;
> > +
> > return 0;
> > cleanup:
> > komeda_plane_destroy(plane);
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-09 5:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20 3:33 [PATCH] drm/komeda: Adds zorder support Lowry Li (Arm Technology China)
2019-05-20 6:05 ` james qian wang (Arm Technology China)
2019-10-08 15:00 ` [PATCH] " Daniel Vetter
2019-10-09 5:15 ` james qian wang (Arm Technology China)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).