No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Changes since v1: - Make __drm_atomic_helper_plane_reset consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel. Drivers already check for that. - Add a proper commit message for driver changes.
Drivers that subclass drm_plane need to copy the logic for linking the drm_plane with its state and to initialize core properties to their default values. E.g (alpha and rotation)
Having a helper to reset the plane_state makes sense because of multiple reasons: 1. Eliminate code duplication. 2. Add a single place for initializing core properties to their default values, no need for driver to do it if what the helper sets makes sense for them. 3. No need to debug the driver when you enable a core property and observe it doesn't have a proper default value.
Tested with mali-dp the other drivers are just built-tested.
Alexandru Gheorghe (10): drm/atomic: Add __drm_atomic_helper_plane_reset drm/amd/display: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/exynos: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vc4: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vmwgfx: Use __drm_atomic_helper_plane_reset instead of copying the logic
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++-- drivers/gpu/drm/arm/malidp_planes.c | 7 ++-- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +-- drivers/gpu/drm/drm_atomic_helper.c | 33 ++++++++++++++----- drivers/gpu/drm/exynos/exynos_drm_plane.c | 3 +- drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++--- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 +-- drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-- drivers/gpu/drm/vc4/vc4_plane.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +-- include/drm/drm_atomic_helper.h | 2 ++ 12 files changed, 41 insertions(+), 47 deletions(-)
There are a lot of drivers that subclass drm_plane_state, all of them duplicate the code that links together the plane with plane_state.
On top of that, drivers that enable core properties also have to duplicate the code for initializing the properties to their default values, which in all cases are the same as the defaults from core.
Change since v1: - Make it consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/drm_atomic_helper.c | 33 +++++++++++++++++++++-------- include/drm/drm_atomic_helper.h | 2 ++ 2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 866a2cc72ef6..6dd5036545ec 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3552,6 +3552,28 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, } EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
+/** + * __drm_atomic_helper_plane_reset - resets planes state to default values + * @plane: plane object, must not be NULL + * @state: atomic plane state, must not be NULL + * + * Initializes plane state to default. This is useful for drivers that subclass + * the plane state. + */ +void __drm_atomic_helper_plane_reset(struct drm_plane *plane, + struct drm_plane_state *state) +{ + state->plane = plane; + state->rotation = DRM_MODE_ROTATE_0; + + /* Reset the alpha value to fully opaque if it matters */ + if (plane->alpha_property) + state->alpha = plane->alpha_property->values[1]; + + plane->state = state; +} +EXPORT_SYMBOL(__drm_atomic_helper_plane_reset); + /** * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for planes * @plane: drm plane @@ -3566,15 +3588,8 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
kfree(plane->state); plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL); - - if (plane->state) { - plane->state->plane = plane; - plane->state->rotation = DRM_MODE_ROTATE_0; - - /* Reset the alpha value to fully opaque if it matters */ - if (plane->alpha_property) - plane->state->alpha = plane->alpha_property->values[1]; - } + if (plane->state) + __drm_atomic_helper_plane_reset(plane, plane->state); } EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 99e2a5297c69..f4c7ed876c97 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -156,6 +156,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state); void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state);
+void __drm_atomic_helper_plane_reset(struct drm_plane *plane, + struct drm_plane_state *state); void drm_atomic_helper_plane_reset(struct drm_plane *plane); void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane, struct drm_plane_state *state);
On Sat, 2018-08-04 at 17:15 +0100, Alexandru Gheorghe wrote:
There are a lot of drivers that subclass drm_plane_state, all of them duplicate the code that links together the plane with plane_state.
On top of that, drivers that enable core properties also have to duplicate the code for initializing the properties to their default values, which in all cases are the same as the defaults from core.
Change since v1:
- Make it consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
regards Philipp
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index ae09331eed00..8e4978d5b83f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2965,11 +2965,8 @@ static void dm_drm_plane_reset(struct drm_plane *plane) amdgpu_state = kzalloc(sizeof(*amdgpu_state), GFP_KERNEL); WARN_ON(amdgpu_state == NULL); - if (amdgpu_state) { - plane->state = &amdgpu_state->base; - plane->state->plane = plane; - plane->state->rotation = DRM_MODE_ROTATE_0; - } + if (amdgpu_state) + __drm_atomic_helper_plane_reset(plane, &amdgpu_state->base); }
static struct drm_plane_state *
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Ayan Kumar halder ayan.halder@arm.com Acked-by: Liviu Dudau liviu.dudau@arm.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/arm/malidp_planes.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 29409a65d864..49c37f6dd63e 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -78,11 +78,8 @@ static void malidp_plane_reset(struct drm_plane *plane) kfree(state); plane->state = NULL; state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { - state->base.plane = plane; - state->base.rotation = DRM_MODE_ROTATE_0; - plane->state = &state->base; - } + if (state) + __drm_atomic_helper_plane_reset(plane, &state->base); }
static struct
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Acked-by: Boris Brezillon boris.brezillon@bootlin.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index 04440064b9b7..9330a076e15a 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -942,10 +942,7 @@ static void atmel_hlcdc_plane_reset(struct drm_plane *p) "Failed to allocate initial plane state\n"); return; } - - p->state = &state->base; - p->state->alpha = DRM_BLEND_ALPHA_OPAQUE; - p->state->plane = p; + __drm_atomic_helper_plane_reset(p, &state->base); } }
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/exynos/exynos_drm_plane.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index eb9915da7dec..681328fbe7de 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -139,8 +139,7 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL); if (exynos_state) { - plane->state = &exynos_state->base; - plane->state->plane = plane; + __drm_atomic_helper_plane_reset(plane, &exynos_state->base); plane->state->zpos = exynos_plane->config->zpos; } }
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/imx/ipuv3-plane.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 203f247d4854..1bd4de03ce9e 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -281,16 +281,14 @@ static void ipu_plane_state_reset(struct drm_plane *plane) ipu_state = to_ipu_plane_state(plane->state); __drm_atomic_helper_plane_destroy_state(plane->state); kfree(ipu_state); + plane->state = NULL; }
ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
- if (ipu_state) { - ipu_state->base.plane = plane; - ipu_state->base.rotation = DRM_MODE_ROTATE_0; - } + if (ipu_state) + __drm_atomic_helper_plane_reset(plane, &ipu_state->base);
- plane->state = &ipu_state->base; }
static struct drm_plane_state *
Hi Alexandru,
On Sat, 2018-08-04 at 17:15 +0100, Alexandru Gheorghe wrote:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
drivers/gpu/drm/imx/ipuv3-plane.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 203f247d4854..1bd4de03ce9e 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -281,16 +281,14 @@ static void ipu_plane_state_reset(struct drm_plane *plane) ipu_state = to_ipu_plane_state(plane->state); __drm_atomic_helper_plane_destroy_state(plane->state); kfree(ipu_state);
plane->state = NULL;}
ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
- if (ipu_state) {
ipu_state->base.plane = plane;ipu_state->base.rotation = DRM_MODE_ROTATE_0;- }
- if (ipu_state)
__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
Could you remove this white line as well?
- plane->state = &ipu_state->base;
}
static struct drm_plane_state *
Acked-by: Philipp Zabel p.zabel@pengutronix.de
to be merged through drm-misc together with the other changes.
regards Philipp
On Mon, Aug 06, 2018 at 08:29:13AM +0200, Philipp Zabel wrote:
Hi,
Hi Alexandru,
On Sat, 2018-08-04 at 17:15 +0100, Alexandru Gheorghe wrote:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
drivers/gpu/drm/imx/ipuv3-plane.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 203f247d4854..1bd4de03ce9e 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -281,16 +281,14 @@ static void ipu_plane_state_reset(struct drm_plane *plane) ipu_state = to_ipu_plane_state(plane->state); __drm_atomic_helper_plane_destroy_state(plane->state); kfree(ipu_state);
plane->state = NULL;}
ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
- if (ipu_state) {
ipu_state->base.plane = plane;ipu_state->base.rotation = DRM_MODE_ROTATE_0;- }
- if (ipu_state)
__drm_atomic_helper_plane_reset(plane, &ipu_state->base);Could you remove this white line as well?
Yes, will do before applying.
- plane->state = &ipu_state->base;
}
static struct drm_plane_state *
Acked-by: Philipp Zabel p.zabel@pengutronix.de
to be merged through drm-misc together with the other changes.
regards Philipp
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++---- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 +---- 2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index c20f7ed48c8d..8861e715c248 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -690,14 +690,12 @@ static void rcar_du_plane_reset(struct drm_plane *plane) if (state == NULL) return;
+ __drm_atomic_helper_plane_reset(plane, &state->state); + state->hwindex = -1; state->source = RCAR_DU_PLANE_MEMORY; state->colorkey = RCAR_DU_COLORKEY_NONE; state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1; - - plane->state = &state->state; - plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE; - plane->state->plane = plane; }
static int rcar_du_plane_atomic_set_property(struct drm_plane *plane, diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 72eebeda518e..45eb777a16a4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -346,11 +346,8 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane) if (state == NULL) return;
- state->state.alpha = DRM_BLEND_ALPHA_OPAQUE; + __drm_atomic_helper_plane_reset(plane, &state->state); state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1; - - plane->state = &state->state; - plane->state->plane = plane; }
static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Acked-by: Maxime Ripard maxime.ripard@bootlin.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c index 750ad24de1d7..78f77af8805a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_layer.c +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c @@ -35,9 +35,7 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
state = kzalloc(sizeof(*state), GFP_KERNEL); if (state) { - plane->state = &state->state; - plane->state->plane = plane; - plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE; + __drm_atomic_helper_plane_reset(plane, &state->state); plane->state->zpos = layer->id; } }
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/vc4/vc4_plane.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 9d7a36f148cf..688ad9bb0f08 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -200,9 +200,7 @@ static void vc4_plane_reset(struct drm_plane *plane) if (!vc4_state) return;
- plane->state = &vc4_state->base; - plane->state->alpha = DRM_BLEND_ALPHA_OPAQUE; - vc4_state->base.plane = plane; + __drm_atomic_helper_plane_reset(plane, &vc4_state->base); }
static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val)
Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com writes:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
Acked-by: Eric Anholt eric@anholt.net
Hi Eric,
On Mon, Aug 06, 2018 at 12:58:20PM -0700, Eric Anholt wrote:
Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com writes:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
__drm_atomic_helper_plane_reset initializes the alpha property to its max value, which is defined by the drm core as DRM_BLEND_ALPHA_OPAQUE, so nothing changes regarding the alpha value.
Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
Acked-by: Eric Anholt eric@anholt.net
Pushed to drmi-misc-next.
-- Cheers, Alex G IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Sinclair Yeh syeh@vmware.com Reviewed-by: Deepak Rawat drawat@vmware.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 4a0f0f41afa1..61824e360619 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -720,9 +720,7 @@ void vmw_du_plane_reset(struct drm_plane *plane) return; }
- plane->state = &vps->base; - plane->state->plane = plane; - plane->state->rotation = DRM_MODE_ROTATE_0; + __drm_atomic_helper_plane_reset(plane, &vps->base); }
Acked-by: Sinclair Yeh syeh@vmware.com
On Sat, Aug 04, 2018 at 05:15:30PM +0100, Alexandru Gheorghe wrote:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Sinclair Yeh syeh@vmware.com Reviewed-by: Deepak Rawat drawat@vmware.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 4a0f0f41afa1..61824e360619 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -720,9 +720,7 @@ void vmw_du_plane_reset(struct drm_plane *plane) return; }
- plane->state = &vps->base;
- plane->state->plane = plane;
- plane->state->rotation = DRM_MODE_ROTATE_0;
- __drm_atomic_helper_plane_reset(plane, &vps->base);
}
-- 2.18.0
Hi Sinclair,
Is it ok if I merge this patch through drm-misc-next ?
Thank you, Alex Gheorghe
On Mon, Aug 06, 2018 at 09:57:53AM -0700, Sinclair Yeh wrote:
Acked-by: Sinclair Yeh syeh@vmware.com
On Sat, Aug 04, 2018 at 05:15:30PM +0100, Alexandru Gheorghe wrote:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Sinclair Yeh syeh@vmware.com Reviewed-by: Deepak Rawat drawat@vmware.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 4a0f0f41afa1..61824e360619 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -720,9 +720,7 @@ void vmw_du_plane_reset(struct drm_plane *plane) return; }
- plane->state = &vps->base;
- plane->state->plane = plane;
- plane->state->rotation = DRM_MODE_ROTATE_0;
- __drm_atomic_helper_plane_reset(plane, &vps->base);
}
-- 2.18.0
On Tue, Aug 07, 2018 at 09:03:27AM +0100, Alexandru-Cosmin Gheorghe wrote:
Hi Sinclair,
Is it ok if I merge this patch through drm-misc-next ?
Sure. Thanks for handling this.
Sinclair
Thank you, Alex Gheorghe
On Mon, Aug 06, 2018 at 09:57:53AM -0700, Sinclair Yeh wrote:
Acked-by: Sinclair Yeh syeh@vmware.com
On Sat, Aug 04, 2018 at 05:15:30PM +0100, Alexandru Gheorghe wrote:
A new helper function(__drm_atomic_helper_plane_reset) has been added for linking a plane with its state and resetting the core properties(alpha, rotation, etc.) to their default values. Use that instead of duplicating the logic.
Reviewed-by: Sinclair Yeh syeh@vmware.com Reviewed-by: Deepak Rawat drawat@vmware.com Signed-off-by: Alexandru Gheorghe alexandru-cosmin.gheorghe@arm.com
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 4a0f0f41afa1..61824e360619 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -720,9 +720,7 @@ void vmw_du_plane_reset(struct drm_plane *plane) return; }
- plane->state = &vps->base;
- plane->state->plane = plane;
- plane->state->rotation = DRM_MODE_ROTATE_0;
- __drm_atomic_helper_plane_reset(plane, &vps->base);
}
-- 2.18.0
-- Cheers, Alex G
Hi,
On Sat, Aug 04, 2018 at 05:15:20PM +0100, Alexandru Gheorghe wrote:
No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Pushed the following patch to drm-misc-next:
drm/atomic: Add __drm_atomic_helper_plane_reset drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic
Thank you, Alex Gheorghe.
Changes since v1:
- Make __drm_atomic_helper_plane_reset consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel. Drivers already check for that.
- Add a proper commit message for driver changes.
Drivers that subclass drm_plane need to copy the logic for linking the drm_plane with its state and to initialize core properties to their default values. E.g (alpha and rotation)
Having a helper to reset the plane_state makes sense because of multiple reasons:
- Eliminate code duplication.
- Add a single place for initializing core properties to their
default values, no need for driver to do it if what the helper sets makes sense for them. 3. No need to debug the driver when you enable a core property and observe it doesn't have a proper default value.
Tested with mali-dp the other drivers are just built-tested.
Alexandru Gheorghe (10): drm/atomic: Add __drm_atomic_helper_plane_reset drm/amd/display: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/exynos: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vc4: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vmwgfx: Use __drm_atomic_helper_plane_reset instead of copying the logic
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++-- drivers/gpu/drm/arm/malidp_planes.c | 7 ++-- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +-- drivers/gpu/drm/drm_atomic_helper.c | 33 ++++++++++++++----- drivers/gpu/drm/exynos/exynos_drm_plane.c | 3 +- drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++--- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 +-- drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-- drivers/gpu/drm/vc4/vc4_plane.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +-- include/drm/drm_atomic_helper.h | 2 ++ 12 files changed, 41 insertions(+), 47 deletions(-)
-- 2.18.0
Hi Alex,
On Monday, 6 August 2018 14:07:27 EEST Alexandru-Cosmin Gheorghe wrote:
Hi,
On Sat, Aug 04, 2018 at 05:15:20PM +0100, Alexandru Gheorghe wrote:
No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Pushed the following patch to drm-misc-next:
drm/atomic: Add __drm_atomic_helper_plane_reset drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic
I've acked the rcar-du part, could it be merged with the rest of the code ?
Changes since v1:
Make __drm_atomic_helper_plane_reset consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel. Drivers already check for that.
Add a proper commit message for driver changes.
Drivers that subclass drm_plane need to copy the logic for linking the drm_plane with its state and to initialize core properties to their default values. E.g (alpha and rotation)
Having a helper to reset the plane_state makes sense because of multiple reasons:
- Eliminate code duplication.
- Add a single place for initializing core properties to their
default values, no need for driver to do it if what the helper sets makes sense for them. 3. No need to debug the driver when you enable a core property and observe it doesn't have a proper default value.
Tested with mali-dp the other drivers are just built-tested.
Alexandru Gheorghe (10): drm/atomic: Add __drm_atomic_helper_plane_reset drm/amd/display: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/exynos: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vc4: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vmwgfx: Use __drm_atomic_helper_plane_reset instead of copying the logic
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++-- drivers/gpu/drm/arm/malidp_planes.c | 7 ++-- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +-- drivers/gpu/drm/drm_atomic_helper.c | 33 ++++++++++++++----- drivers/gpu/drm/exynos/exynos_drm_plane.c | 3 +- drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++--- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 +-- drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-- drivers/gpu/drm/vc4/vc4_plane.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +-- include/drm/drm_atomic_helper.h | 2 ++ 12 files changed, 41 insertions(+), 47 deletions(-)
On Mon, Aug 06, 2018 at 02:45:42PM +0300, Laurent Pinchart wrote:
Hi Laurent,
Hi Alex,
On Monday, 6 August 2018 14:07:27 EEST Alexandru-Cosmin Gheorghe wrote:
Hi,
On Sat, Aug 04, 2018 at 05:15:20PM +0100, Alexandru Gheorghe wrote:
No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Pushed the following patch to drm-misc-next:
drm/atomic: Add __drm_atomic_helper_plane_reset drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic
I've acked the rcar-du part, could it be merged with the rest of the code ?
Done, it's in drm-misc-next now. I didn't know how to handle this for drivers that have their own tree, so I just pushed the patches where I was explicitly asked.
Thank you, Alex Gheorghe
Changes since v1:
Make __drm_atomic_helper_plane_reset consistent with the other helpers and require that both plane and state not be NULL, suggested by Boris Brezillon and Philipp Zabel. Drivers already check for that.
Add a proper commit message for driver changes.
Drivers that subclass drm_plane need to copy the logic for linking the drm_plane with its state and to initialize core properties to their default values. E.g (alpha and rotation)
Having a helper to reset the plane_state makes sense because of multiple reasons:
- Eliminate code duplication.
- Add a single place for initializing core properties to their
default values, no need for driver to do it if what the helper sets makes sense for them. 3. No need to debug the driver when you enable a core property and observe it doesn't have a proper default value.
Tested with mali-dp the other drivers are just built-tested.
Alexandru Gheorghe (10): drm/atomic: Add __drm_atomic_helper_plane_reset drm/amd/display: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/exynos: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: rcar-du: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vc4: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/vmwgfx: Use __drm_atomic_helper_plane_reset instead of copying the logic
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++-- drivers/gpu/drm/arm/malidp_planes.c | 7 ++-- .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +-- drivers/gpu/drm/drm_atomic_helper.c | 33 ++++++++++++++----- drivers/gpu/drm/exynos/exynos_drm_plane.c | 3 +- drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++--- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 6 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 5 +-- drivers/gpu/drm/sun4i/sun4i_layer.c | 4 +-- drivers/gpu/drm/vc4/vc4_plane.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 +-- include/drm/drm_atomic_helper.h | 2 ++ 12 files changed, 41 insertions(+), 47 deletions(-)
-- Regards,
Laurent Pinchart
Hi Alex,
On Monday, 6 August 2018 15:20:38 EEST Alexandru-Cosmin Gheorghe wrote:
On Mon, Aug 06, 2018 at 02:45:42PM +0300, Laurent Pinchart wrote:
On Monday, 6 August 2018 14:07:27 EEST Alexandru-Cosmin Gheorghe wrote:
On Sat, Aug 04, 2018 at 05:15:20PM +0100, Alexandru Gheorghe wrote:
No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Pushed the following patch to drm-misc-next:
drm/atomic: Add __drm_atomic_helper_plane_reset drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic
I've acked the rcar-du part, could it be merged with the rest of the code ?
Done, it's in drm-misc-next now. I didn't know how to handle this for drivers that have their own tree, so I just pushed the patches where I was explicitly asked.
No worries. It's actually better to ask than pushing changes directly, as maintainers could have conflicting patches queued up. I should have made this clear in my review, thank you for handling it.
On Mon, Aug 06, 2018 at 03:28:06PM +0300, Laurent Pinchart wrote:
Hi Alex,
On Monday, 6 August 2018 15:20:38 EEST Alexandru-Cosmin Gheorghe wrote:
On Mon, Aug 06, 2018 at 02:45:42PM +0300, Laurent Pinchart wrote:
On Monday, 6 August 2018 14:07:27 EEST Alexandru-Cosmin Gheorghe wrote:
On Sat, Aug 04, 2018 at 05:15:20PM +0100, Alexandru Gheorghe wrote:
No significant change since v2, fixed a spelling mistake and added/removed some newlines in 01 and 07 patches.
I plan to apply the first patch of the series and the patches for the drivers maintained through drm-misc(that go Reviewed/Ack) in drm-misc-next on Monday.
For the other drivers please let me know if you want me to push them in drm-misc-next as well.
Pushed the following patch to drm-misc-next:
drm/atomic: Add __drm_atomic_helper_plane_reset drm: mali-dp: Use __drm_atomic_helper_plane_reset instead of copying the logic drm: atmel-hlcdc: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/imx: Use __drm_atomic_helper_plane_reset instead of copying the logic drm/sun4i: Use __drm_atomic_helper_plane_reset instead of copying the logic
I've acked the rcar-du part, could it be merged with the rest of the code ?
Done, it's in drm-misc-next now. I didn't know how to handle this for drivers that have their own tree, so I just pushed the patches where I was explicitly asked.
No worries. It's actually better to ask than pushing changes directly, as maintainers could have conflicting patches queued up. I should have made this clear in my review, thank you for handling it.
Poke them a few times, if no response then just get someone to review the remaining ones and push them all into drm-misc-next. This is needed all the time because many drivers aren't group maintained and the single maintainer swamped/absent. Worst case there's a conflict or a double-merge, which isn't really all that bad. -Daniel
dri-devel@lists.freedesktop.org
-
Alexandru Gheorghe -
Alexandru-Cosmin Gheorghe
-
Daniel Vetter -
Eric Anholt -
Laurent Pinchart -
Philipp Zabel -
Sinclair Yeh