* [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
@ 2017-11-01 18:29 Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
` (8 more replies)
0 siblings, 9 replies; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
While trawling the tree I spotted some issues with the way vmwgfx
uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
Do note that I haven't actually tested the resulting code at all,
but it does build at least.
And while touching that general area I took up Daniel's suggestion from
long ago that drm_plane_helper_check_state() should be renamed and
relocated to better reflect its status.
Here's a branch with the entire series:
git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Ville Syrjälä (5):
drm/vmwgfx: Remove bogus crtc coords vs fb size check
drm/vmwgfx: Use drm_plane_helper_check_state()
drm/vmwgfx: Try to fix plane clipping
drm: Check crtc_state->enable rather than crtc->enabled in
drm_plane_helper_check_state()
drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +-
drivers/gpu/drm/arm/malidp_planes.c | 3 +-
drivers/gpu/drm/drm_atomic_helper.c | 95 ++++++++++++++++++++++++
drivers/gpu/drm/drm_plane_helper.c | 111 +++-------------------------
drivers/gpu/drm/drm_simple_kms_helper.c | 9 ++-
drivers/gpu/drm/i915/intel_display.c | 20 ++---
drivers/gpu/drm/imx/ipuv3-plane.c | 8 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +-
drivers/gpu/drm/meson/meson_plane.c | 8 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
drivers/gpu/drm/nouveau/nv50_display.c | 18 +++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
drivers/gpu/drm/tegra/dc.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 ++++------
drivers/gpu/drm/zte/zx_plane.c | 15 ++--
include/drm/drm_atomic_helper.h | 7 ++
include/drm/drm_plane_helper.h | 5 --
17 files changed, 187 insertions(+), 183 deletions(-)
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
@ 2017-11-01 18:29 ` Ville Syrjala
2017-11-02 10:04 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
` (7 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
define the viewport inside the fb, that's a job for the src coords,
which have been checked by the core already.
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 0545740b3724..a4b56699679a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
vcs = vmw_connector_state_to_vcs(du->connector.state);
- if ((dest.x2 > new_fb->width ||
- dest.y2 > new_fb->height)) {
- DRM_ERROR("CRTC area outside of framebuffer\n");
- return -EINVAL;
- }
-
/* Only one active implicit framebuffer at a time. */
mutex_lock(&dev_priv->global_kms_state_mutex);
if (vcs->is_implicit && dev_priv->implicit_fb &&
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
@ 2017-11-01 18:29 ` Ville Syrjala
2017-11-02 10:06 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
` (6 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Atomic drivers have no reason to use drm_plane_helper_check_update()
instead of drm_plane_helper_check_state(). So let's switch over.
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index a4b56699679a..515b67783a41 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
struct drm_framebuffer *new_fb = state->fb;
- bool visible;
-
- struct drm_rect src = {
- .x1 = state->src_x,
- .y1 = state->src_y,
- .x2 = state->src_x + state->src_w,
- .y2 = state->src_y + state->src_h,
- };
- struct drm_rect dest = {
+ struct drm_rect clip = {
.x1 = state->crtc_x,
.y1 = state->crtc_y,
.x2 = state->crtc_x + state->crtc_w,
.y2 = state->crtc_y + state->crtc_h,
};
- struct drm_rect clip = dest;
int ret;
- ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
- &src, &dest, &clip,
- DRM_MODE_ROTATE_0,
+ ret = drm_plane_helper_check_state(state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
- false, true, &visible);
+ false, true);
if (!ret && new_fb) {
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
@ 2017-11-01 18:29 ` Ville Syrjala
2017-11-02 10:12 ` Daniel Vetter
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
` (5 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, Sinclair Yeh
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Try to fix the code to actually clip the plane to the crtc bounds
instead of the user provided crtc coordinates (which would be a no-op
since those are exactly the coordinates before clipping).
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 515b67783a41..efa41c086198 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
+ struct drm_crtc_state *crtc_state = NULL;
struct drm_framebuffer *new_fb = state->fb;
- struct drm_rect clip = {
- .x1 = state->crtc_x,
- .y1 = state->crtc_y,
- .x2 = state->crtc_x + state->crtc_w,
- .y2 = state->crtc_y + state->crtc_h,
- };
+ struct drm_rect clip = {};
int ret;
- ret = drm_plane_helper_check_state(state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ if (state->crtc)
+ crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
+ if (crtc_state && crtc_state->enable) {
+ clip.x2 = crtc_state->adjusted_mode.hdisplay;
+ clip.y2 = crtc_state->adjusted_mode.vdisplay;
+ }
+
+ ret = drm_plane_helper_check_state(state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (!ret && new_fb) {
struct drm_crtc *crtc = state->crtc;
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (2 preceding siblings ...)
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
@ 2017-11-01 18:29 ` Ville Syrjala
2017-11-01 20:15 ` [PATCH v2 " Ville Syrjala
2017-11-23 9:52 ` [PATCH " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
` (4 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_plane_helper_check_state() is supposed to do things the atomic way,
so it should not be inspecting crtc->enabled. Rather we should be
looking at crtc_state->enable.
We have a slight complication due to drm_plane_helper_check_update()
reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
we'll have to pass the crtc_state in manally and construct a fake
crtc_state in drm_plane_helper_check_update().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_planes.c | 3 +-
drivers/gpu/drm/drm_plane_helper.c | 52 +++++++++++++++++------------
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 2 ++
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +--
drivers/gpu/drm/nouveau/nv50_display.c | 6 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/tegra/dc.c | 4 +--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/zte/zx_plane.c | 4 +--
include/drm/drm_plane_helper.h | 3 +-
15 files changed, 53 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 72b22b805412..14721723fa8a 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 94e7e3fa3408..492d99dd55d4 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 759ed93f4ba8..adb8d94484f2 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
/**
* drm_plane_helper_check_state() - Check plane state for validity
- * @state: plane state to check
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
* @clip: integer clipping coordinates
* @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
* @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
@@ -120,35 +121,38 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
* RETURNS:
* Zero if update appears valid, error code on failure
*/
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
const struct drm_rect *clip,
int min_scale,
int max_scale,
bool can_position,
bool can_update_disabled)
{
- struct drm_crtc *crtc = state->crtc;
- struct drm_framebuffer *fb = state->fb;
- struct drm_rect *src = &state->src;
- struct drm_rect *dst = &state->dst;
- unsigned int rotation = state->rotation;
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect *src = &plane_state->src;
+ struct drm_rect *dst = &plane_state->dst;
+ unsigned int rotation = plane_state->rotation;
int hscale, vscale;
- *src = drm_plane_state_src(state);
- *dst = drm_plane_state_dest(state);
+ WARN_ON(!crtc_state != !plane_state->crtc);
+ WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
+
+ *src = drm_plane_state_src(plane_state);
+ *dst = drm_plane_state_dest(plane_state);
if (!fb) {
- state->visible = false;
+ plane_state->visible = false;
return 0;
}
/* crtc should only be NULL when disabling (i.e., !fb) */
- if (WARN_ON(!crtc)) {
- state->visible = false;
+ if (WARN_ON(!plane_state->crtc)) {
+ plane_state->visible = false;
return 0;
}
- if (!crtc->enabled && !can_update_disabled) {
+ if (!crtc_state->enable && !can_update_disabled) {
DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}
@@ -160,16 +164,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
DRM_DEBUG_KMS("Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", &state->src, true);
- drm_rect_debug_print("dst: ", &state->dst, false);
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
}
- state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+ plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
- if (!state->visible)
+ if (!plane_state->visible)
/*
* Plane isn't visible; some drivers can handle this
* so we just return success here. Drivers that can't
@@ -230,7 +234,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
bool can_update_disabled,
bool *visible)
{
- struct drm_plane_state state = {
+ struct drm_plane_state plane_state = {
.plane = plane,
.crtc = crtc,
.fb = fb,
@@ -245,18 +249,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
.rotation = rotation,
.visible = *visible,
};
+ struct drm_crtc_state crtc_state = {
+ .crtc = crtc,
+ .enable = crtc->enabled,
+ };
int ret;
- ret = drm_plane_helper_check_state(&state, clip,
+ ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
min_scale, max_scale,
can_position,
can_update_disabled);
if (ret)
return ret;
- *src = state.src;
- *dst = state.dst;
- *visible = state.visible;
+ *src = plane_state.src;
+ *dst = plane_state.dst;
+ *visible = plane_state.visible;
return 0;
}
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index dc9fd109de14..d428c805025c 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(plane_state, &clip,
+ ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a459726b36b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
int ret;
ret = drm_plane_helper_check_state(&plane_state->base,
+ &crtc_state->base,
&plane_state->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
@@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
}
ret = drm_plane_helper_check_state(&state->base,
+ &crtc_state->base,
&state->clip,
min_scale, max_scale,
can_position, true);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 247c60e6bed2..51b23ac175e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
clip.y1 = 0;
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
can_position, true);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 6f121891430f..7ebb33657704 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,7 +111,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 17e96fa47868..2a00b720c371 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 4b22ac3413a1..1c194a9453d9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
min_scale = FRAC_16_16(1, 8);
max_scale = FRAC_16_16(8, 1);
- ret = drm_plane_helper_check_state(state, &clip, min_scale,
- max_scale, true, true);
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+ min_scale, max_scale, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e4751f92b342..7f11a88d0593 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
{
int ret;
- ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+ ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+ &asyw->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
@@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
if (!fb->format->depth)
return -EINVAL;
- ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+ ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+ &asyw->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..36d0f101e30d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
min_scale, max_scale,
true, true);
if (ret)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 24a5ef4f5bb8..e95d9e2a7f6c 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
clip.y2 = crtc_state->mode.vdisplay;
/* Check plane state for visibility and calculate clipping bounds */
- err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
- true, true);
+ err = drm_plane_helper_check_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (err < 0)
return err;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efa41c086198..d95e7b1c3b11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
clip.y2 = crtc_state->adjusted_mode.vdisplay;
}
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 18e763493264..ee0002529b8f 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, &clip,
+ return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
min_scale, max_scale,
true, true);
}
@@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, &clip,
+ return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 7c8a00ceadb7..41b8309b0a57 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,7 +38,8 @@
*/
#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
const struct drm_rect *clip,
int min_scale, int max_scale,
bool can_position,
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (3 preceding siblings ...)
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
@ 2017-11-01 18:29 ` Ville Syrjala
2017-11-01 20:16 ` [PATCH v2 " Ville Syrjala
2017-11-23 9:53 ` [PATCH " Laurent Pinchart
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
` (3 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 18:29 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_plane_helper_check_update() isn't a transitional helper, so let's
rename it to drm_atomic_helper_check_plane_state() and move it into
drm_atomic_helper.c.
Cc: Daniel Vetter <daniel@ffwll.ch>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +--
drivers/gpu/drm/arm/malidp_planes.c | 4 +-
drivers/gpu/drm/drm_atomic_helper.c | 95 +++++++++++++++++++++++++
drivers/gpu/drm/drm_plane_helper.c | 103 ++--------------------------
drivers/gpu/drm/drm_simple_kms_helper.c | 9 +--
drivers/gpu/drm/i915/intel_display.c | 22 +++---
drivers/gpu/drm/imx/ipuv3-plane.c | 8 +--
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +--
drivers/gpu/drm/meson/meson_plane.c | 8 +--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
drivers/gpu/drm/nouveau/nv50_display.c | 20 +++---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
drivers/gpu/drm/tegra/dc.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 +--
drivers/gpu/drm/zte/zx_plane.c | 15 ++--
include/drm/drm_atomic_helper.h | 7 ++
include/drm/drm_plane_helper.h | 6 --
17 files changed, 170 insertions(+), 166 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 14721723fa8a..63511a3bbf6c 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
}
static void hdlcd_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 492d99dd55d4..72a07950167e 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- 0, INT_MAX, true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..7595ad8ad2f3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -696,6 +696,101 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
/**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ * doesn't cover the entire crtc? This will generally
+ * only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ * is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled)
+{
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect *src = &plane_state->src;
+ struct drm_rect *dst = &plane_state->dst;
+ unsigned int rotation = plane_state->rotation;
+ int hscale, vscale;
+
+ WARN_ON(!crtc_state != !plane_state->crtc);
+ WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
+
+ *src = drm_plane_state_src(plane_state);
+ *dst = drm_plane_state_dest(plane_state);
+
+ if (!fb) {
+ plane_state->visible = false;
+ return 0;
+ }
+
+ /* crtc should only be NULL when disabling (i.e., !fb) */
+ if (WARN_ON(!plane_state->crtc)) {
+ plane_state->visible = false;
+ return 0;
+ }
+
+ if (!crtc_state->enable && !can_update_disabled) {
+ DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+ return -EINVAL;
+ }
+
+ drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+
+ /* Check scaling */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+ if (hscale < 0 || vscale < 0) {
+ DRM_DEBUG_KMS("Invalid scaling of plane\n");
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
+ return -ERANGE;
+ }
+
+ plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+
+ drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+
+ if (!plane_state->visible)
+ /*
+ * Plane isn't visible; some drivers can handle this
+ * so we just return success here. Drivers that can't
+ * (including those that use the primary plane helper's
+ * update function) will return an error from their
+ * update_plane handler.
+ */
+ return 0;
+
+ if (!can_position && !drm_rect_equals(dst, clip)) {
+ DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+ drm_rect_debug_print("dst: ", dst, false);
+ drm_rect_debug_print("clip: ", clip, false);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
+
+/**
* drm_atomic_helper_check_planes - validate state object for planes changes
* @dev: DRM device
* @state: the driver state object
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index adb8d94484f2..f1be8cd4e387 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -100,101 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
- * drm_plane_helper_check_state() - Check plane state for validity
- * @plane_state: plane state to check
- * @crtc_state: crtc state to check
- * @clip: integer clipping coordinates
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
- * @can_position: is it legal to position the plane such that it
- * doesn't cover the entire crtc? This will generally
- * only be false for primary planes.
- * @can_update_disabled: can the plane be updated while the crtc
- * is disabled?
- *
- * Checks that a desired plane update is valid, and updates various
- * bits of derived state (clipped coordinates etc.). Drivers that provide
- * their own plane handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * RETURNS:
- * Zero if update appears valid, error code on failure
- */
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- const struct drm_rect *clip,
- int min_scale,
- int max_scale,
- bool can_position,
- bool can_update_disabled)
-{
- struct drm_framebuffer *fb = plane_state->fb;
- struct drm_rect *src = &plane_state->src;
- struct drm_rect *dst = &plane_state->dst;
- unsigned int rotation = plane_state->rotation;
- int hscale, vscale;
-
- WARN_ON(!crtc_state != !plane_state->crtc);
- WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
-
- *src = drm_plane_state_src(plane_state);
- *dst = drm_plane_state_dest(plane_state);
-
- if (!fb) {
- plane_state->visible = false;
- return 0;
- }
-
- /* crtc should only be NULL when disabling (i.e., !fb) */
- if (WARN_ON(!plane_state->crtc)) {
- plane_state->visible = false;
- return 0;
- }
-
- if (!crtc_state->enable && !can_update_disabled) {
- DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
- return -EINVAL;
- }
-
- drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
-
- /* Check scaling */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (hscale < 0 || vscale < 0) {
- DRM_DEBUG_KMS("Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", &plane_state->src, true);
- drm_rect_debug_print("dst: ", &plane_state->dst, false);
- return -ERANGE;
- }
-
- plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
-
- drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
-
- if (!plane_state->visible)
- /*
- * Plane isn't visible; some drivers can handle this
- * so we just return success here. Drivers that can't
- * (including those that use the primary plane helper's
- * update function) will return an error from their
- * update_plane handler.
- */
- return 0;
-
- if (!can_position && !drm_rect_equals(dst, clip)) {
- DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
- drm_rect_debug_print("dst: ", dst, false);
- drm_rect_debug_print("clip: ", clip, false);
- return -EINVAL;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(drm_plane_helper_check_state);
-
-/**
* drm_plane_helper_check_update() - Check plane update for validity
* @plane: plane object to update
* @crtc: owning CRTC of owning plane
@@ -255,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
};
int ret;
- ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
- min_scale, max_scale,
- can_position,
- can_update_disabled);
+ ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
+ clip, min_scale, max_scale,
+ can_position,
+ can_update_disabled);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index d428c805025c..9f3b1c94802b 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4a459726b36b..e0ff599d2090 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
u32 offset;
int ret;
- ret = drm_plane_helper_check_state(&plane_state->base,
- &crtc_state->base,
- &plane_state->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(&plane_state->base,
+ &crtc_state->base,
+ &plane_state->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
if (ret)
return ret;
@@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
can_position = true;
}
- ret = drm_plane_helper_check_state(&state->base,
- &crtc_state->base,
- &state->clip,
- min_scale, max_scale,
- can_position, true);
+ ret = drm_atomic_helper_check_plane_state(&state->base,
+ &crtc_state->base,
+ &state->clip,
+ min_scale, max_scale,
+ can_position, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 51b23ac175e3..5a67daedcf4d 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
clip.y1 = 0;
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- can_position, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ can_position, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 7ebb33657704..5ef898b93d8d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,10 +111,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
}
static void mtk_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 2a00b720c371..d0a6ac8390f3 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
}
/* Takes a fixed 16.16 number and converts it to integer. */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 1c194a9453d9..df3360acacca 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
min_scale = FRAC_16_16(1, 8);
max_scale = FRAC_16_16(8, 1);
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- min_scale, max_scale, true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ min_scale, max_scale,
+ true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 7f11a88d0593..8ab0fac3544d 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
{
int ret;
- ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
- &asyw->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+ &asyw->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
asyh->curs.visible = asyw->state.visible;
if (ret || !asyh->curs.visible)
return ret;
@@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
if (!fb->format->depth)
return -EINVAL;
- ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
- &asyw->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+ &asyw->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 36d0f101e30d..ba7505292b78 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- min_scale, max_scale,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ min_scale, max_scale,
+ true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index e95d9e2a7f6c..fc70351b9017 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
clip.y2 = crtc_state->mode.vdisplay;
/* Check plane state for visibility and calculate clipping bounds */
- err = drm_plane_helper_check_state(state, crtc_state, &clip,
- 0, INT_MAX, true, true);
+ err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (err < 0)
return err;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d95e7b1c3b11..a2a93d7e2a04 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
clip.y2 = crtc_state->adjusted_mode.vdisplay;
}
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (!ret && new_fb) {
struct drm_crtc *crtc = state->crtc;
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index ee0002529b8f..68fd2e2dc78a 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- min_scale, max_scale,
- true, true);
+ return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip, min_scale, max_scale,
+ true, true);
}
static int zx_vl_get_fmt(uint32_t format)
@@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
}
static int zx_gl_get_fmt(uint32_t format)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d2b56cc657e9..4842ee9485ce 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -38,6 +38,13 @@ struct drm_private_state;
int drm_atomic_helper_check_modeset(struct drm_device *dev,
struct drm_atomic_state *state);
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled);
int drm_atomic_helper_check_planes(struct drm_device *dev,
struct drm_atomic_state *state);
int drm_atomic_helper_check(struct drm_device *dev,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 41b8309b0a57..8aa49c0ecd4d 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,12 +38,6 @@
*/
#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- const struct drm_rect *clip,
- int min_scale, int max_scale,
- bool can_position,
- bool can_update_disabled);
int drm_plane_helper_check_update(struct drm_plane *plane,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (4 preceding siblings ...)
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
@ 2017-11-01 19:46 ` Patchwork
2017-11-01 20:10 ` Ville Syrjälä
2017-11-01 21:02 ` ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3) Patchwork
` (2 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Patchwork @ 2017-11-01 19:46 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm: drm_plane_helper_check_state() related stuff
URL : https://patchwork.freedesktop.org/series/33002/
State : failure
== Summary ==
Series 33002v1 drm: drm_plane_helper_check_state() related stuff
https://patchwork.freedesktop.org/api/1.0/series/33002/revisions/1/mbox/
Test chamelium:
Subgroup dp-edid-read:
pass -> DMESG-WARN (fi-kbl-7500u) fdo#102672
Subgroup dp-crc-fast:
pass -> DMESG-WARN (fi-kbl-7500u) fdo#102514
Subgroup hdmi-edid-read:
pass -> DMESG-WARN (fi-skl-6700k)
Subgroup hdmi-crc-fast:
pass -> DMESG-WARN (fi-skl-6700k)
Subgroup common-hpd-after-suspend:
pass -> DMESG-WARN (fi-skl-6700k)
Test debugfs_test:
Subgroup read_all_entries:
pass -> DMESG-WARN (fi-gdg-551)
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
pass -> DMESG-WARN (fi-bwr-2160)
pass -> DMESG-WARN (fi-elk-e7500)
pass -> DMESG-WARN (fi-ilk-650)
pass -> DMESG-WARN (fi-snb-2520m)
pass -> DMESG-WARN (fi-snb-2600)
pass -> DMESG-WARN (fi-ivb-3520m)
pass -> DMESG-WARN (fi-ivb-3770)
pass -> DMESG-WARN (fi-byt-j1900)
pass -> DMESG-WARN (fi-byt-n2820)
pass -> DMESG-WARN (fi-hsw-4770)
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-bsw-n3050)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u) fdo#103285
pass -> DMESG-WARN (fi-kbl-7560u)
pass -> DMESG-WARN (fi-kbl-7567u)
pass -> DMESG-WARN (fi-kbl-r)
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-glk-dsi)
pass -> DMESG-WARN (fi-cfl-s) fdo#103186
pass -> DMESG-WARN (fi-cnl-y)
Test drv_hangman:
Subgroup error-state-basic:
pass -> DMESG-WARN (fi-gdg-551)
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
pass -> DMESG-WARN (fi-bwr-2160)
Test gem_busy:
Subgroup basic-hang-default:
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_fence:
Subgroup await-hang-default:
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
pass -> DMESG-WARN (fi-elk-e7500)
pass -> DMESG-WARN (fi-ilk-650)
pass -> DMESG-WARN (fi-snb-2520m)
pass -> DMESG-WARN (fi-snb-2600) fdo#102365
pass -> DMESG-WARN (fi-ivb-3520m)
pass -> DMESG-WARN (fi-ivb-3770)
pass -> DMESG-WARN (fi-byt-j1900)
pass -> DMESG-WARN (fi-byt-n2820)
pass -> DMESG-WARN (fi-hsw-4770)
pass -> DMESG-WARN (fi-hsw-4770r)
pass -> DMESG-WARN (fi-bdw-5557u)
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-bsw-n3050)
pass -> DMESG-WARN (fi-skl-6260u)
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-6700hq)
pass -> DMESG-WARN (fi-skl-6700k)
pass -> DMESG-WARN (fi-skl-6770hq)
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
pass -> DMESG-WARN (fi-bxt-j4205)
pass -> DMESG-WARN (fi-kbl-7500u)
pass -> DMESG-FAIL (fi-kbl-7560u) fdo#103039
pass -> DMESG-WARN (fi-kbl-7567u) fdo#102846 +11
pass -> DMESG-WARN (fi-glk-1)
pass -> DMESG-WARN (fi-glk-dsi)
pass -> DMESG-WARN (fi-cnl-y)
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-blb-e6850)
pass -> DMESG-WARN (fi-pnv-d510)
pass -> DMESG-WARN (fi-elk-e7500)
pass -> DMESG-WARN (fi-ilk-650)
pass -> DMESG-WARN (fi-snb-2520m)
WARNING: Long output truncated
7da46c0a019ebaec3f67a8a6ccc60a56c2d521b1 drm-tip: 2017y-11m-01d-17h-32m-50s UTC integration manifest
6d81deae6f47 drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
a9ff8519c323 drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
67d75a7c518d drm/vmwgfx: Try to fix plane clipping
b824112d55f1 drm/vmwgfx: Use drm_plane_helper_check_state()
9e9194d33463 drm/vmwgfx: Remove bogus crtc coords vs fb size check
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6302/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
@ 2017-11-01 20:10 ` Ville Syrjälä
0 siblings, 0 replies; 31+ messages in thread
From: Ville Syrjälä @ 2017-11-01 20:10 UTC (permalink / raw)
To: intel-gfx
On Wed, Nov 01, 2017 at 07:46:18PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm: drm_plane_helper_check_state() related stuff
> URL : https://patchwork.freedesktop.org/series/33002/
> State : failure
>
> == Summary ==
>
> Series 33002v1 drm: drm_plane_helper_check_state() related stuff
> https://patchwork.freedesktop.org/api/1.0/series/33002/revisions/1/mbox/
>
> Test chamelium:
> Subgroup dp-edid-read:
> pass -> DMESG-WARN (fi-kbl-7500u) fdo#102672
> Subgroup dp-crc-fast:
> pass -> DMESG-WARN (fi-kbl-7500u) fdo#102514
> Subgroup hdmi-edid-read:
> pass -> DMESG-WARN (fi-skl-6700k)
> Subgroup hdmi-crc-fast:
> pass -> DMESG-WARN (fi-skl-6700k)
> Subgroup common-hpd-after-suspend:
> pass -> DMESG-WARN (fi-skl-6700k)
Thinko in my WARNs.
I guess it needs to be something like:
- WARN_ON(!crtc_state != !plane_state->crtc);
- WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
+ WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
since we grab the crtc from the old plane state if the new plane state
doesn't have one. And thus the crtc state we pass around may refer to
the old crtc, not the new one (well, there won't be a new one in
those cases).
Although I'm not sure how sensible it is to use the crtc state of the
old crtc for state computation/checks in general. Feels a bit iffy
at least.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
@ 2017-11-01 20:15 ` Ville Syrjala
2017-11-02 10:15 ` Daniel Vetter
2017-11-23 9:52 ` [PATCH " Laurent Pinchart
1 sibling, 1 reply; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 20:15 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_plane_helper_check_state() is supposed to do things the atomic way,
so it should not be inspecting crtc->enabled. Rather we should be
looking at crtc_state->enable.
We have a slight complication due to drm_plane_helper_check_update()
reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
we'll have to pass the crtc_state in manally and construct a fake
crtc_state in drm_plane_helper_check_update().
v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_planes.c | 3 +-
drivers/gpu/drm/drm_plane_helper.c | 51 ++++++++++++++++-------------
drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 2 ++
drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
drivers/gpu/drm/meson/meson_plane.c | 2 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +--
drivers/gpu/drm/nouveau/nv50_display.c | 6 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
drivers/gpu/drm/tegra/dc.c | 4 +--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
drivers/gpu/drm/zte/zx_plane.c | 4 +--
include/drm/drm_plane_helper.h | 3 +-
15 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 72b22b805412..14721723fa8a 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 94e7e3fa3408..492d99dd55d4 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 759ed93f4ba8..eef0ffcd7ed5 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
/**
* drm_plane_helper_check_state() - Check plane state for validity
- * @state: plane state to check
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
* @clip: integer clipping coordinates
* @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
* @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
@@ -120,35 +121,37 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
* RETURNS:
* Zero if update appears valid, error code on failure
*/
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
const struct drm_rect *clip,
int min_scale,
int max_scale,
bool can_position,
bool can_update_disabled)
{
- struct drm_crtc *crtc = state->crtc;
- struct drm_framebuffer *fb = state->fb;
- struct drm_rect *src = &state->src;
- struct drm_rect *dst = &state->dst;
- unsigned int rotation = state->rotation;
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect *src = &plane_state->src;
+ struct drm_rect *dst = &plane_state->dst;
+ unsigned int rotation = plane_state->rotation;
int hscale, vscale;
- *src = drm_plane_state_src(state);
- *dst = drm_plane_state_dest(state);
+ WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
+
+ *src = drm_plane_state_src(plane_state);
+ *dst = drm_plane_state_dest(plane_state);
if (!fb) {
- state->visible = false;
+ plane_state->visible = false;
return 0;
}
/* crtc should only be NULL when disabling (i.e., !fb) */
- if (WARN_ON(!crtc)) {
- state->visible = false;
+ if (WARN_ON(!plane_state->crtc)) {
+ plane_state->visible = false;
return 0;
}
- if (!crtc->enabled && !can_update_disabled) {
+ if (!crtc_state->enable && !can_update_disabled) {
DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
return -EINVAL;
}
@@ -160,16 +163,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
if (hscale < 0 || vscale < 0) {
DRM_DEBUG_KMS("Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", &state->src, true);
- drm_rect_debug_print("dst: ", &state->dst, false);
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
return -ERANGE;
}
- state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+ plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
- if (!state->visible)
+ if (!plane_state->visible)
/*
* Plane isn't visible; some drivers can handle this
* so we just return success here. Drivers that can't
@@ -230,7 +233,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
bool can_update_disabled,
bool *visible)
{
- struct drm_plane_state state = {
+ struct drm_plane_state plane_state = {
.plane = plane,
.crtc = crtc,
.fb = fb,
@@ -245,18 +248,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
.rotation = rotation,
.visible = *visible,
};
+ struct drm_crtc_state crtc_state = {
+ .crtc = crtc,
+ .enable = crtc->enabled,
+ };
int ret;
- ret = drm_plane_helper_check_state(&state, clip,
+ ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
min_scale, max_scale,
can_position,
can_update_disabled);
if (ret)
return ret;
- *src = state.src;
- *dst = state.dst;
- *visible = state.visible;
+ *src = plane_state.src;
+ *dst = plane_state.dst;
+ *visible = plane_state.visible;
return 0;
}
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index dc9fd109de14..d428c805025c 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(plane_state, &clip,
+ ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a459726b36b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
int ret;
ret = drm_plane_helper_check_state(&plane_state->base,
+ &crtc_state->base,
&plane_state->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
@@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
}
ret = drm_plane_helper_check_state(&state->base,
+ &crtc_state->base,
&state->clip,
min_scale, max_scale,
can_position, true);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 247c60e6bed2..51b23ac175e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
clip.y1 = 0;
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
can_position, true);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 6f121891430f..7ebb33657704 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,7 +111,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 17e96fa47868..2a00b720c371 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, &clip,
+ return drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 4b22ac3413a1..1c194a9453d9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
min_scale = FRAC_16_16(1, 8);
max_scale = FRAC_16_16(8, 1);
- ret = drm_plane_helper_check_state(state, &clip, min_scale,
- max_scale, true, true);
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+ min_scale, max_scale, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e4751f92b342..7f11a88d0593 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
{
int ret;
- ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+ ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+ &asyw->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
true, true);
@@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
if (!fb->format->depth)
return -EINVAL;
- ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+ ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+ &asyw->clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..36d0f101e30d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
min_scale, max_scale,
true, true);
if (ret)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 24a5ef4f5bb8..e95d9e2a7f6c 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
clip.y2 = crtc_state->mode.vdisplay;
/* Check plane state for visibility and calculate clipping bounds */
- err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
- true, true);
+ err = drm_plane_helper_check_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (err < 0)
return err;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efa41c086198..d95e7b1c3b11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
clip.y2 = crtc_state->adjusted_mode.vdisplay;
}
- ret = drm_plane_helper_check_state(state, &clip,
+ ret = drm_plane_helper_check_state(state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 18e763493264..ee0002529b8f 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, &clip,
+ return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
min_scale, max_scale,
true, true);
}
@@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, &clip,
+ return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 7c8a00ceadb7..41b8309b0a57 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,7 +38,8 @@
*/
#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
const struct drm_rect *clip,
int min_scale, int max_scale,
bool can_position,
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
@ 2017-11-01 20:16 ` Ville Syrjala
2017-11-02 10:16 ` Daniel Vetter
2017-11-23 9:53 ` [PATCH " Laurent Pinchart
1 sibling, 1 reply; 31+ messages in thread
From: Ville Syrjala @ 2017-11-01 20:16 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_plane_helper_check_update() isn't a transitional helper, so let's
rename it to drm_atomic_helper_check_plane_state() and move it into
drm_atomic_helper.c.
v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
Cc: Daniel Vetter <daniel@ffwll.ch>
Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +--
drivers/gpu/drm/arm/malidp_planes.c | 4 +-
drivers/gpu/drm/drm_atomic_helper.c | 94 +++++++++++++++++++++++++
drivers/gpu/drm/drm_plane_helper.c | 102 ++--------------------------
drivers/gpu/drm/drm_simple_kms_helper.c | 9 +--
drivers/gpu/drm/i915/intel_display.c | 22 +++---
drivers/gpu/drm/imx/ipuv3-plane.c | 8 +--
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +--
drivers/gpu/drm/meson/meson_plane.c | 8 +--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
drivers/gpu/drm/nouveau/nv50_display.c | 20 +++---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
drivers/gpu/drm/tegra/dc.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 +--
drivers/gpu/drm/zte/zx_plane.c | 15 ++--
include/drm/drm_atomic_helper.h | 7 ++
include/drm/drm_plane_helper.h | 6 --
17 files changed, 169 insertions(+), 165 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 14721723fa8a..63511a3bbf6c 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
}
static void hdlcd_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 492d99dd55d4..72a07950167e 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- 0, INT_MAX, true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..a381913c894d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -696,6 +696,100 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
/**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
+ * @clip: integer clipping coordinates
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ * doesn't cover the entire crtc? This will generally
+ * only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the crtc
+ * is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled)
+{
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect *src = &plane_state->src;
+ struct drm_rect *dst = &plane_state->dst;
+ unsigned int rotation = plane_state->rotation;
+ int hscale, vscale;
+
+ WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
+
+ *src = drm_plane_state_src(plane_state);
+ *dst = drm_plane_state_dest(plane_state);
+
+ if (!fb) {
+ plane_state->visible = false;
+ return 0;
+ }
+
+ /* crtc should only be NULL when disabling (i.e., !fb) */
+ if (WARN_ON(!plane_state->crtc)) {
+ plane_state->visible = false;
+ return 0;
+ }
+
+ if (!crtc_state->enable && !can_update_disabled) {
+ DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
+ return -EINVAL;
+ }
+
+ drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+
+ /* Check scaling */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
+ if (hscale < 0 || vscale < 0) {
+ DRM_DEBUG_KMS("Invalid scaling of plane\n");
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
+ return -ERANGE;
+ }
+
+ plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+
+ drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+
+ if (!plane_state->visible)
+ /*
+ * Plane isn't visible; some drivers can handle this
+ * so we just return success here. Drivers that can't
+ * (including those that use the primary plane helper's
+ * update function) will return an error from their
+ * update_plane handler.
+ */
+ return 0;
+
+ if (!can_position && !drm_rect_equals(dst, clip)) {
+ DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
+ drm_rect_debug_print("dst: ", dst, false);
+ drm_rect_debug_print("clip: ", clip, false);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
+
+/**
* drm_atomic_helper_check_planes - validate state object for planes changes
* @dev: DRM device
* @state: the driver state object
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index eef0ffcd7ed5..f1be8cd4e387 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -100,100 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
}
/**
- * drm_plane_helper_check_state() - Check plane state for validity
- * @plane_state: plane state to check
- * @crtc_state: crtc state to check
- * @clip: integer clipping coordinates
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
- * @can_position: is it legal to position the plane such that it
- * doesn't cover the entire crtc? This will generally
- * only be false for primary planes.
- * @can_update_disabled: can the plane be updated while the crtc
- * is disabled?
- *
- * Checks that a desired plane update is valid, and updates various
- * bits of derived state (clipped coordinates etc.). Drivers that provide
- * their own plane handling rather than helper-provided implementations may
- * still wish to call this function to avoid duplication of error checking
- * code.
- *
- * RETURNS:
- * Zero if update appears valid, error code on failure
- */
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- const struct drm_rect *clip,
- int min_scale,
- int max_scale,
- bool can_position,
- bool can_update_disabled)
-{
- struct drm_framebuffer *fb = plane_state->fb;
- struct drm_rect *src = &plane_state->src;
- struct drm_rect *dst = &plane_state->dst;
- unsigned int rotation = plane_state->rotation;
- int hscale, vscale;
-
- WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
-
- *src = drm_plane_state_src(plane_state);
- *dst = drm_plane_state_dest(plane_state);
-
- if (!fb) {
- plane_state->visible = false;
- return 0;
- }
-
- /* crtc should only be NULL when disabling (i.e., !fb) */
- if (WARN_ON(!plane_state->crtc)) {
- plane_state->visible = false;
- return 0;
- }
-
- if (!crtc_state->enable && !can_update_disabled) {
- DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
- return -EINVAL;
- }
-
- drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
-
- /* Check scaling */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (hscale < 0 || vscale < 0) {
- DRM_DEBUG_KMS("Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", &plane_state->src, true);
- drm_rect_debug_print("dst: ", &plane_state->dst, false);
- return -ERANGE;
- }
-
- plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
-
- drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
-
- if (!plane_state->visible)
- /*
- * Plane isn't visible; some drivers can handle this
- * so we just return success here. Drivers that can't
- * (including those that use the primary plane helper's
- * update function) will return an error from their
- * update_plane handler.
- */
- return 0;
-
- if (!can_position && !drm_rect_equals(dst, clip)) {
- DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
- drm_rect_debug_print("dst: ", dst, false);
- drm_rect_debug_print("clip: ", clip, false);
- return -EINVAL;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(drm_plane_helper_check_state);
-
-/**
* drm_plane_helper_check_update() - Check plane update for validity
* @plane: plane object to update
* @crtc: owning CRTC of owning plane
@@ -254,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
};
int ret;
- ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
- min_scale, max_scale,
- can_position,
- can_update_disabled);
+ ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
+ clip, min_scale, max_scale,
+ can_position,
+ can_update_disabled);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index d428c805025c..9f3b1c94802b 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4a459726b36b..e0ff599d2090 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
u32 offset;
int ret;
- ret = drm_plane_helper_check_state(&plane_state->base,
- &crtc_state->base,
- &plane_state->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(&plane_state->base,
+ &crtc_state->base,
+ &plane_state->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
if (ret)
return ret;
@@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
can_position = true;
}
- ret = drm_plane_helper_check_state(&state->base,
- &crtc_state->base,
- &state->clip,
- min_scale, max_scale,
- can_position, true);
+ ret = drm_atomic_helper_check_plane_state(&state->base,
+ &crtc_state->base,
+ &state->clip,
+ min_scale, max_scale,
+ can_position, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 51b23ac175e3..5a67daedcf4d 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
clip.y1 = 0;
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- can_position, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ can_position, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 7ebb33657704..5ef898b93d8d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,10 +111,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
}
static void mtk_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 2a00b720c371..d0a6ac8390f3 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->mode.hdisplay;
clip.y2 = crtc_state->mode.vdisplay;
- return drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
}
/* Takes a fixed 16.16 number and converts it to integer. */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 1c194a9453d9..df3360acacca 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
min_scale = FRAC_16_16(1, 8);
max_scale = FRAC_16_16(8, 1);
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- min_scale, max_scale, true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ min_scale, max_scale,
+ true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 7f11a88d0593..8ab0fac3544d 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
{
int ret;
- ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
- &asyw->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+ &asyw->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
asyh->curs.visible = asyw->state.visible;
if (ret || !asyh->curs.visible)
return ret;
@@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
if (!fb->format->depth)
return -EINVAL;
- ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
- &asyw->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
+ &asyw->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 36d0f101e30d..ba7505292b78 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- min_scale, max_scale,
- true, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ min_scale, max_scale,
+ true, true);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index e95d9e2a7f6c..fc70351b9017 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
clip.y2 = crtc_state->mode.vdisplay;
/* Check plane state for visibility and calculate clipping bounds */
- err = drm_plane_helper_check_state(state, crtc_state, &clip,
- 0, INT_MAX, true, true);
+ err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ 0, INT_MAX, true, true);
if (err < 0)
return err;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d95e7b1c3b11..a2a93d7e2a04 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
clip.y2 = crtc_state->adjusted_mode.vdisplay;
}
- ret = drm_plane_helper_check_state(state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
if (!ret && new_fb) {
struct drm_crtc *crtc = state->crtc;
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index ee0002529b8f..68fd2e2dc78a 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- min_scale, max_scale,
- true, true);
+ return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip, min_scale, max_scale,
+ true, true);
}
static int zx_vl_get_fmt(uint32_t format)
@@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
clip.x2 = crtc_state->adjusted_mode.hdisplay;
clip.y2 = crtc_state->adjusted_mode.vdisplay;
- return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true);
+ return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
}
static int zx_gl_get_fmt(uint32_t format)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d2b56cc657e9..4842ee9485ce 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -38,6 +38,13 @@ struct drm_private_state;
int drm_atomic_helper_check_modeset(struct drm_device *dev,
struct drm_atomic_state *state);
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ const struct drm_rect *clip,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled);
int drm_atomic_helper_check_planes(struct drm_device *dev,
struct drm_atomic_state *state);
int drm_atomic_helper_check(struct drm_device *dev,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 41b8309b0a57..8aa49c0ecd4d 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,12 +38,6 @@
*/
#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
-int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- const struct drm_rect *clip,
- int min_scale, int max_scale,
- bool can_position,
- bool can_update_disabled);
int drm_plane_helper_check_update(struct drm_plane *plane,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 31+ messages in thread
* ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3)
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (5 preceding siblings ...)
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
@ 2017-11-01 21:02 ` Patchwork
2017-11-01 21:52 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
8 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2017-11-01 21:02 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm: drm_plane_helper_check_state() related stuff (rev3)
URL : https://patchwork.freedesktop.org/series/33002/
State : success
== Summary ==
Series 33002v3 drm: drm_plane_helper_check_state() related stuff
https://patchwork.freedesktop.org/api/1.0/series/33002/revisions/3/mbox/
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Subgroup common-hpd-after-suspend:
dmesg-warn -> PASS (fi-kbl-7500u) fdo#102505
Test kms_frontbuffer_tracking:
Subgroup basic:
fail -> PASS (fi-glk-dsi) fdo#103167
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:442s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:459s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:378s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:539s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:512s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:500s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:510s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:487s
fi-cfl-s total:289 pass:253 dwarn:4 dfail:0 fail:0 skip:32 time:546s
fi-cnl-y total:217 pass:196 dwarn:0 dfail:0 fail:0 skip:20
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:421s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:269s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:586s
fi-glk-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:489s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:434s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:428s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:424s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:498s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:465s
fi-kbl-7500u total:289 pass:264 dwarn:0 dfail:0 fail:1 skip:24 time:475s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:575s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:478s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:580s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:574s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:457s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:593s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:652s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:507s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:571s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:426s
7da46c0a019ebaec3f67a8a6ccc60a56c2d521b1 drm-tip: 2017y-11m-01d-17h-32m-50s UTC integration manifest
5fd2ee28cd29 drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
f5a1386e738b drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
9b967ad53783 drm/vmwgfx: Try to fix plane clipping
6fb4866d13f0 drm/vmwgfx: Use drm_plane_helper_check_state()
488c96ed9533 drm/vmwgfx: Remove bogus crtc coords vs fb size check
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6304/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* ✓ Fi.CI.IGT: success for drm: drm_plane_helper_check_state() related stuff (rev3)
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (6 preceding siblings ...)
2017-11-01 21:02 ` ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3) Patchwork
@ 2017-11-01 21:52 ` Patchwork
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
8 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2017-11-01 21:52 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm: drm_plane_helper_check_state() related stuff (rev3)
URL : https://patchwork.freedesktop.org/series/33002/
State : success
== Summary ==
Test drv_module_reload:
Subgroup basic-reload:
pass -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_flip:
Subgroup modeset-vs-vblank-race-interruptible:
fail -> PASS (shard-hsw) fdo#103060
Test perf:
Subgroup oa-exponents:
fail -> PASS (shard-hsw) fdo#102254
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
shard-hsw total:2539 pass:1431 dwarn:2 dfail:0 fail:9 skip:1097 time:9286s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6304/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
@ 2017-11-02 10:04 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-11-02 10:04 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Wed, Nov 01, 2017 at 08:29:16PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
> define the viewport inside the fb, that's a job for the src coords,
> which have been checked by the core already.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 0545740b3724..a4b56699679a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>
> vcs = vmw_connector_state_to_vcs(du->connector.state);
>
> - if ((dest.x2 > new_fb->width ||
> - dest.y2 > new_fb->height)) {
> - DRM_ERROR("CRTC area outside of framebuffer\n");
> - return -EINVAL;
> - }
> -
> /* Only one active implicit framebuffer at a time. */
> mutex_lock(&dev_priv->global_kms_state_mutex);
> if (vcs->is_implicit && dev_priv->implicit_fb &&
> --
> 2.13.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
@ 2017-11-02 10:06 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-11-02 10:06 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Wed, Nov 01, 2017 at 08:29:17PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Atomic drivers have no reason to use drm_plane_helper_check_update()
> instead of drm_plane_helper_check_state(). So let's switch over.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I think the new atomic helper landed at about the same time vmwgfx landed,
hence why it wasn't converted.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index a4b56699679a..515b67783a41 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> struct drm_framebuffer *new_fb = state->fb;
> - bool visible;
> -
> - struct drm_rect src = {
> - .x1 = state->src_x,
> - .y1 = state->src_y,
> - .x2 = state->src_x + state->src_w,
> - .y2 = state->src_y + state->src_h,
> - };
> - struct drm_rect dest = {
> + struct drm_rect clip = {
> .x1 = state->crtc_x,
> .y1 = state->crtc_y,
> .x2 = state->crtc_x + state->crtc_w,
> .y2 = state->crtc_y + state->crtc_h,
> };
> - struct drm_rect clip = dest;
> int ret;
>
> - ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
> - &src, &dest, &clip,
> - DRM_MODE_ROTATE_0,
> + ret = drm_plane_helper_check_state(state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> - false, true, &visible);
> + false, true);
>
>
> if (!ret && new_fb) {
> --
> 2.13.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
@ 2017-11-02 10:12 ` Daniel Vetter
2017-11-02 13:19 ` Ville Syrjälä
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-11-02 10:12 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Try to fix the code to actually clip the plane to the crtc bounds
> instead of the user provided crtc coordinates (which would be a no-op
> since those are exactly the coordinates before clipping).
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I kinda wonder whether we shouldn't push this down into the helper ...
Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 515b67783a41..efa41c086198 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> + struct drm_crtc_state *crtc_state = NULL;
> struct drm_framebuffer *new_fb = state->fb;
> - struct drm_rect clip = {
> - .x1 = state->crtc_x,
> - .y1 = state->crtc_y,
> - .x2 = state->crtc_x + state->crtc_w,
> - .y2 = state->crtc_y + state->crtc_h,
> - };
> + struct drm_rect clip = {};
> int ret;
>
> - ret = drm_plane_helper_check_state(state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + if (state->crtc)
> + crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
>
> + if (crtc_state && crtc_state->enable) {
> + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> + clip.y2 = crtc_state->adjusted_mode.vdisplay;
> + }
> +
> + ret = drm_plane_helper_check_state(state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
>
> if (!ret && new_fb) {
> struct drm_crtc *crtc = state->crtc;
> --
> 2.13.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
2017-11-01 20:15 ` [PATCH v2 " Ville Syrjala
@ 2017-11-02 10:15 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-11-02 10:15 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, dri-devel
On Wed, Nov 01, 2017 at 10:15:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_plane_helper_check_state() is supposed to do things the atomic way,
> so it should not be inspecting crtc->enabled. Rather we should be
> looking at crtc_state->enable.
Oh dear, how often are those legacy state thingies going to hurt us :-/
> We have a slight complication due to drm_plane_helper_check_update()
> reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
> we'll have to pass the crtc_state in manally and construct a fake
> crtc_state in drm_plane_helper_check_update().
>
> v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I guess even more reasons to drop the clip rectangle and just look at
crtc_state?
Anyway, this looks good to me.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
> drivers/gpu/drm/arm/malidp_planes.c | 3 +-
> drivers/gpu/drm/drm_plane_helper.c | 51 ++++++++++++++++-------------
> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
> drivers/gpu/drm/meson/meson_plane.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +--
> drivers/gpu/drm/nouveau/nv50_display.c | 6 ++--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
> drivers/gpu/drm/tegra/dc.c | 4 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
> drivers/gpu/drm/zte/zx_plane.c | 4 +--
> include/drm/drm_plane_helper.h | 3 +-
> 15 files changed, 52 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 72b22b805412..14721723fa8a 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 94e7e3fa3408..492d99dd55d4 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
>
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 759ed93f4ba8..eef0ffcd7ed5 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>
> /**
> * drm_plane_helper_check_state() - Check plane state for validity
> - * @state: plane state to check
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> * @clip: integer clipping coordinates
> * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> @@ -120,35 +121,37 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> * RETURNS:
> * Zero if update appears valid, error code on failure
> */
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> const struct drm_rect *clip,
> int min_scale,
> int max_scale,
> bool can_position,
> bool can_update_disabled)
> {
> - struct drm_crtc *crtc = state->crtc;
> - struct drm_framebuffer *fb = state->fb;
> - struct drm_rect *src = &state->src;
> - struct drm_rect *dst = &state->dst;
> - unsigned int rotation = state->rotation;
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_rect *src = &plane_state->src;
> + struct drm_rect *dst = &plane_state->dst;
> + unsigned int rotation = plane_state->rotation;
> int hscale, vscale;
>
> - *src = drm_plane_state_src(state);
> - *dst = drm_plane_state_dest(state);
> + WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> +
> + *src = drm_plane_state_src(plane_state);
> + *dst = drm_plane_state_dest(plane_state);
>
> if (!fb) {
> - state->visible = false;
> + plane_state->visible = false;
> return 0;
> }
>
> /* crtc should only be NULL when disabling (i.e., !fb) */
> - if (WARN_ON(!crtc)) {
> - state->visible = false;
> + if (WARN_ON(!plane_state->crtc)) {
> + plane_state->visible = false;
> return 0;
> }
>
> - if (!crtc->enabled && !can_update_disabled) {
> + if (!crtc_state->enable && !can_update_disabled) {
> DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> return -EINVAL;
> }
> @@ -160,16 +163,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
> vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> if (hscale < 0 || vscale < 0) {
> DRM_DEBUG_KMS("Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", &state->src, true);
> - drm_rect_debug_print("dst: ", &state->dst, false);
> + drm_rect_debug_print("src: ", &plane_state->src, true);
> + drm_rect_debug_print("dst: ", &plane_state->dst, false);
> return -ERANGE;
> }
>
> - state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> + plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>
> drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
>
> - if (!state->visible)
> + if (!plane_state->visible)
> /*
> * Plane isn't visible; some drivers can handle this
> * so we just return success here. Drivers that can't
> @@ -230,7 +233,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> bool can_update_disabled,
> bool *visible)
> {
> - struct drm_plane_state state = {
> + struct drm_plane_state plane_state = {
> .plane = plane,
> .crtc = crtc,
> .fb = fb,
> @@ -245,18 +248,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> .rotation = rotation,
> .visible = *visible,
> };
> + struct drm_crtc_state crtc_state = {
> + .crtc = crtc,
> + .enable = crtc->enabled,
> + };
> int ret;
>
> - ret = drm_plane_helper_check_state(&state, clip,
> + ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> min_scale, max_scale,
> can_position,
> can_update_disabled);
> if (ret)
> return ret;
>
> - *src = state.src;
> - *dst = state.dst;
> - *visible = state.visible;
> + *src = plane_state.src;
> + *dst = plane_state.dst;
> + *visible = plane_state.visible;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index dc9fd109de14..d428c805025c 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(plane_state, &clip,
> + ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 737de251d0f8..4a459726b36b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> int ret;
>
> ret = drm_plane_helper_check_state(&plane_state->base,
> + &crtc_state->base,
> &plane_state->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> @@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
> }
>
> ret = drm_plane_helper_check_state(&state->base,
> + &crtc_state->base,
> &state->clip,
> min_scale, max_scale,
> can_position, true);
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 247c60e6bed2..51b23ac175e3 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
> clip.y1 = 0;
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> can_position, true);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 6f121891430f..7ebb33657704 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,7 +111,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 17e96fa47868..2a00b720c371 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 4b22ac3413a1..1c194a9453d9 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
> min_scale = FRAC_16_16(1, 8);
> max_scale = FRAC_16_16(8, 1);
>
> - ret = drm_plane_helper_check_state(state, &clip, min_scale,
> - max_scale, true, true);
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> + min_scale, max_scale, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index e4751f92b342..7f11a88d0593 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
> {
> int ret;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> + ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> @@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
> if (!fb->format->depth)
> return -EINVAL;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> + ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 19128b4dea54..36d0f101e30d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> min_scale, max_scale,
> true, true);
> if (ret)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 24a5ef4f5bb8..e95d9e2a7f6c 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
> clip.y2 = crtc_state->mode.vdisplay;
>
> /* Check plane state for visibility and calculate clipping bounds */
> - err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
> - true, true);
> + err = drm_plane_helper_check_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (err < 0)
> return err;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index efa41c086198..d95e7b1c3b11 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> }
>
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index 18e763493264..ee0002529b8f 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, &clip,
> + return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> min_scale, max_scale,
> true, true);
> }
> @@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, &clip,
> + return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 7c8a00ceadb7..41b8309b0a57 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,7 +38,8 @@
> */
> #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> const struct drm_rect *clip,
> int min_scale, int max_scale,
> bool can_position,
> --
> 2.13.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
2017-11-01 20:16 ` [PATCH v2 " Ville Syrjala
@ 2017-11-02 10:16 ` Daniel Vetter
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-11-02 10:16 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, dri-devel
On Wed, Nov 01, 2017 at 10:16:19PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_plane_helper_check_update() isn't a transitional helper, so let's
> rename it to drm_atomic_helper_check_plane_state() and move it into
> drm_atomic_helper.c.
>
> v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I guess I should finally unlazy and split up drm_atomic_helper.c into the
different parts, it's getting a bit huge. But after your stuff has landed.
Before pushing pls make sure vmwgfx has been tested, and ofc that our CI
approves too.
Cheers, Daniel
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +--
> drivers/gpu/drm/arm/malidp_planes.c | 4 +-
> drivers/gpu/drm/drm_atomic_helper.c | 94 +++++++++++++++++++++++++
> drivers/gpu/drm/drm_plane_helper.c | 102 ++--------------------------
> drivers/gpu/drm/drm_simple_kms_helper.c | 9 +--
> drivers/gpu/drm/i915/intel_display.c | 22 +++---
> drivers/gpu/drm/imx/ipuv3-plane.c | 8 +--
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +--
> drivers/gpu/drm/meson/meson_plane.c | 8 +--
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 20 +++---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
> drivers/gpu/drm/tegra/dc.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 +--
> drivers/gpu/drm/zte/zx_plane.c | 15 ++--
> include/drm/drm_atomic_helper.h | 7 ++
> include/drm/drm_plane_helper.h | 6 --
> 17 files changed, 169 insertions(+), 165 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 14721723fa8a..63511a3bbf6c 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> }
>
> static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index 492d99dd55d4..72a07950167e 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
>
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - 0, INT_MAX, true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 71d712f1b56a..a381913c894d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -696,6 +696,100 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>
> /**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> + * @clip: integer clipping coordinates
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + * doesn't cover the entire crtc? This will generally
> + * only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the crtc
> + * is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> + const struct drm_rect *clip,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled)
> +{
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_rect *src = &plane_state->src;
> + struct drm_rect *dst = &plane_state->dst;
> + unsigned int rotation = plane_state->rotation;
> + int hscale, vscale;
> +
> + WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> +
> + *src = drm_plane_state_src(plane_state);
> + *dst = drm_plane_state_dest(plane_state);
> +
> + if (!fb) {
> + plane_state->visible = false;
> + return 0;
> + }
> +
> + /* crtc should only be NULL when disabling (i.e., !fb) */
> + if (WARN_ON(!plane_state->crtc)) {
> + plane_state->visible = false;
> + return 0;
> + }
> +
> + if (!crtc_state->enable && !can_update_disabled) {
> + DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> + return -EINVAL;
> + }
> +
> + drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> +
> + /* Check scaling */
> + hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> + vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> + if (hscale < 0 || vscale < 0) {
> + DRM_DEBUG_KMS("Invalid scaling of plane\n");
> + drm_rect_debug_print("src: ", &plane_state->src, true);
> + drm_rect_debug_print("dst: ", &plane_state->dst, false);
> + return -ERANGE;
> + }
> +
> + plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> +
> + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> +
> + if (!plane_state->visible)
> + /*
> + * Plane isn't visible; some drivers can handle this
> + * so we just return success here. Drivers that can't
> + * (including those that use the primary plane helper's
> + * update function) will return an error from their
> + * update_plane handler.
> + */
> + return 0;
> +
> + if (!can_position && !drm_rect_equals(dst, clip)) {
> + DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> + drm_rect_debug_print("dst: ", dst, false);
> + drm_rect_debug_print("clip: ", clip, false);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> +
> +/**
> * drm_atomic_helper_check_planes - validate state object for planes changes
> * @dev: DRM device
> * @state: the driver state object
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index eef0ffcd7ed5..f1be8cd4e387 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -100,100 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> }
>
> /**
> - * drm_plane_helper_check_state() - Check plane state for validity
> - * @plane_state: plane state to check
> - * @crtc_state: crtc state to check
> - * @clip: integer clipping coordinates
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> - * @can_position: is it legal to position the plane such that it
> - * doesn't cover the entire crtc? This will generally
> - * only be false for primary planes.
> - * @can_update_disabled: can the plane be updated while the crtc
> - * is disabled?
> - *
> - * Checks that a desired plane update is valid, and updates various
> - * bits of derived state (clipped coordinates etc.). Drivers that provide
> - * their own plane handling rather than helper-provided implementations may
> - * still wish to call this function to avoid duplication of error checking
> - * code.
> - *
> - * RETURNS:
> - * Zero if update appears valid, error code on failure
> - */
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> - const struct drm_crtc_state *crtc_state,
> - const struct drm_rect *clip,
> - int min_scale,
> - int max_scale,
> - bool can_position,
> - bool can_update_disabled)
> -{
> - struct drm_framebuffer *fb = plane_state->fb;
> - struct drm_rect *src = &plane_state->src;
> - struct drm_rect *dst = &plane_state->dst;
> - unsigned int rotation = plane_state->rotation;
> - int hscale, vscale;
> -
> - WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> -
> - *src = drm_plane_state_src(plane_state);
> - *dst = drm_plane_state_dest(plane_state);
> -
> - if (!fb) {
> - plane_state->visible = false;
> - return 0;
> - }
> -
> - /* crtc should only be NULL when disabling (i.e., !fb) */
> - if (WARN_ON(!plane_state->crtc)) {
> - plane_state->visible = false;
> - return 0;
> - }
> -
> - if (!crtc_state->enable && !can_update_disabled) {
> - DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> - return -EINVAL;
> - }
> -
> - drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> -
> - /* Check scaling */
> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> - if (hscale < 0 || vscale < 0) {
> - DRM_DEBUG_KMS("Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", &plane_state->src, true);
> - drm_rect_debug_print("dst: ", &plane_state->dst, false);
> - return -ERANGE;
> - }
> -
> - plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> -
> - drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> -
> - if (!plane_state->visible)
> - /*
> - * Plane isn't visible; some drivers can handle this
> - * so we just return success here. Drivers that can't
> - * (including those that use the primary plane helper's
> - * update function) will return an error from their
> - * update_plane handler.
> - */
> - return 0;
> -
> - if (!can_position && !drm_rect_equals(dst, clip)) {
> - DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> - drm_rect_debug_print("dst: ", dst, false);
> - drm_rect_debug_print("clip: ", clip, false);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_plane_helper_check_state);
> -
> -/**
> * drm_plane_helper_check_update() - Check plane update for validity
> * @plane: plane object to update
> * @crtc: owning CRTC of owning plane
> @@ -254,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> };
> int ret;
>
> - ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> - min_scale, max_scale,
> - can_position,
> - can_update_disabled);
> + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> + clip, min_scale, max_scale,
> + can_position,
> + can_update_disabled);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index d428c805025c..9f3b1c94802b 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4a459726b36b..e0ff599d2090 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
> u32 offset;
> int ret;
>
> - ret = drm_plane_helper_check_state(&plane_state->base,
> - &crtc_state->base,
> - &plane_state->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(&plane_state->base,
> + &crtc_state->base,
> + &plane_state->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> if (ret)
> return ret;
>
> @@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane *plane,
> can_position = true;
> }
>
> - ret = drm_plane_helper_check_state(&state->base,
> - &crtc_state->base,
> - &state->clip,
> - min_scale, max_scale,
> - can_position, true);
> + ret = drm_atomic_helper_check_plane_state(&state->base,
> + &crtc_state->base,
> + &state->clip,
> + min_scale, max_scale,
> + can_position, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 51b23ac175e3..5a67daedcf4d 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
> clip.y1 = 0;
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - can_position, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + can_position, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 7ebb33657704..5ef898b93d8d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,10 +111,10 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> }
>
> static void mtk_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 2a00b720c371..d0a6ac8390f3 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> }
>
> /* Takes a fixed 16.16 number and converts it to integer. */
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 1c194a9453d9..df3360acacca 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
> min_scale = FRAC_16_16(1, 8);
> max_scale = FRAC_16_16(8, 1);
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - min_scale, max_scale, true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + min_scale, max_scale,
> + true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 7f11a88d0593..8ab0fac3544d 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
> {
> int ret;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> - &asyw->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> asyh->curs.visible = asyw->state.visible;
> if (ret || !asyh->curs.visible)
> return ret;
> @@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
> if (!fb->format->depth)
> return -EINVAL;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> - &asyw->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 36d0f101e30d..ba7505292b78 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - min_scale, max_scale,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + min_scale, max_scale,
> + true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index e95d9e2a7f6c..fc70351b9017 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
> clip.y2 = crtc_state->mode.vdisplay;
>
> /* Check plane state for visibility and calculate clipping bounds */
> - err = drm_plane_helper_check_state(state, crtc_state, &clip,
> - 0, INT_MAX, true, true);
> + err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (err < 0)
> return err;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index d95e7b1c3b11..a2a93d7e2a04 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> }
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
>
> if (!ret && new_fb) {
> struct drm_crtc *crtc = state->crtc;
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index ee0002529b8f..68fd2e2dc78a 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - min_scale, max_scale,
> - true, true);
> + return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip, min_scale, max_scale,
> + true, true);
> }
>
> static int zx_vl_get_fmt(uint32_t format)
> @@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> }
>
> static int zx_gl_get_fmt(uint32_t format)
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d2b56cc657e9..4842ee9485ce 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,13 @@ struct drm_private_state;
>
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> + const struct drm_rect *clip,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled);
> int drm_atomic_helper_check_planes(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_atomic_helper_check(struct drm_device *dev,
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 41b8309b0a57..8aa49c0ecd4d 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,12 +38,6 @@
> */
> #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> - const struct drm_crtc_state *crtc_state,
> - const struct drm_rect *clip,
> - int min_scale, int max_scale,
> - bool can_position,
> - bool can_update_disabled);
> int drm_plane_helper_check_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> --
> 2.13.6
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-02 10:12 ` Daniel Vetter
@ 2017-11-02 13:19 ` Ville Syrjälä
2017-11-06 18:04 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 1 reply; 31+ messages in thread
From: Ville Syrjälä @ 2017-11-02 13:19 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Try to fix the code to actually clip the plane to the crtc bounds
> > instead of the user provided crtc coordinates (which would be a no-op
> > since those are exactly the coordinates before clipping).
> >
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I kinda wonder whether we shouldn't push this down into the helper ...
Would require quite going through all drivers making sure they are
happy with using the adjusted_mode.crtc_ timings. I think most drivers
use the other adjusted_mode timings currently, and some even use the
user mode timings (which could be an actual bug).
>
> Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 515b67783a41..efa41c086198 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > struct drm_plane_state *state)
> > {
> > + struct drm_crtc_state *crtc_state = NULL;
> > struct drm_framebuffer *new_fb = state->fb;
> > - struct drm_rect clip = {
> > - .x1 = state->crtc_x,
> > - .y1 = state->crtc_y,
> > - .x2 = state->crtc_x + state->crtc_w,
> > - .y2 = state->crtc_y + state->crtc_h,
> > - };
> > + struct drm_rect clip = {};
> > int ret;
> >
> > - ret = drm_plane_helper_check_state(state, &clip,
> > - DRM_PLANE_HELPER_NO_SCALING,
> > - DRM_PLANE_HELPER_NO_SCALING,
> > - false, true);
> > + if (state->crtc)
> > + crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> >
> > + if (crtc_state && crtc_state->enable) {
> > + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > + clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > + }
> > +
> > + ret = drm_plane_helper_check_state(state, &clip,
> > + DRM_PLANE_HELPER_NO_SCALING,
> > + DRM_PLANE_HELPER_NO_SCALING,
> > + false, true);
> >
> > if (!ret && new_fb) {
> > struct drm_crtc *crtc = state->crtc;
> > --
> > 2.13.6
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-02 13:19 ` Ville Syrjälä
@ 2017-11-06 18:04 ` Ville Syrjälä
2017-11-07 12:30 ` Daniel Vetter
2017-11-23 9:46 ` [Intel-gfx] " Laurent Pinchart
0 siblings, 2 replies; 31+ messages in thread
From: Ville Syrjälä @ 2017-11-06 18:04 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Try to fix the code to actually clip the plane to the crtc bounds
> > > instead of the user provided crtc coordinates (which would be a no-op
> > > since those are exactly the coordinates before clipping).
> > >
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > I kinda wonder whether we shouldn't push this down into the helper ...
>
> Would require quite going through all drivers making sure they are
> happy with using the adjusted_mode.crtc_ timings. I think most drivers
> use the other adjusted_mode timings currently, and some even use the
> user mode timings (which could be an actual bug).
Let me take that back. What we want is to clip to the user mode
timings. Stereo 3D needs the special treatment from
drm_mode_get_hv_timing(). I'm getting confused by all these
different timings we have all over the place.
I think for i915 all we would need is to change the double wide/dual
link adjustent for pipe_src_w to simply reject odd widths instead.
That would guarantee that the user mode timings match the pipe_src_w/h
100%.
For the other driver we'd need to figure out why they're using
adjusted_mode timings for clipping. I guess that's just a mistake,
which I repeated here with vmwgfx after getting confused by looking
at the other drivers.
I guess I just volunteered myself to do this. Just needs plenty of
acks from driver maintainers I suppose.
>
> >
> > Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > > ---
> > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > > 1 file changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > index 515b67783a41..efa41c086198 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > > int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > > struct drm_plane_state *state)
> > > {
> > > + struct drm_crtc_state *crtc_state = NULL;
> > > struct drm_framebuffer *new_fb = state->fb;
> > > - struct drm_rect clip = {
> > > - .x1 = state->crtc_x,
> > > - .y1 = state->crtc_y,
> > > - .x2 = state->crtc_x + state->crtc_w,
> > > - .y2 = state->crtc_y + state->crtc_h,
> > > - };
> > > + struct drm_rect clip = {};
> > > int ret;
> > >
> > > - ret = drm_plane_helper_check_state(state, &clip,
> > > - DRM_PLANE_HELPER_NO_SCALING,
> > > - DRM_PLANE_HELPER_NO_SCALING,
> > > - false, true);
> > > + if (state->crtc)
> > > + crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> > >
> > > + if (crtc_state && crtc_state->enable) {
> > > + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > + clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > + }
> > > +
> > > + ret = drm_plane_helper_check_state(state, &clip,
> > > + DRM_PLANE_HELPER_NO_SCALING,
> > > + DRM_PLANE_HELPER_NO_SCALING,
> > > + false, true);
> > >
> > > if (!ret && new_fb) {
> > > struct drm_crtc *crtc = state->crtc;
> > > --
> > > 2.13.6
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-06 18:04 ` [Intel-gfx] " Ville Syrjälä
@ 2017-11-07 12:30 ` Daniel Vetter
2017-11-23 9:46 ` [Intel-gfx] " Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2017-11-07 12:30 UTC (permalink / raw)
To: Ville Syrjälä
Cc: dri-devel, intel-gfx, VMware Graphics, Thomas Hellstrom
On Mon, Nov 06, 2017 at 08:04:38PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> > On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > Try to fix the code to actually clip the plane to the crtc bounds
> > > > instead of the user provided crtc coordinates (which would be a no-op
> > > > since those are exactly the coordinates before clipping).
> > > >
> > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > I kinda wonder whether we shouldn't push this down into the helper ...
> >
> > Would require quite going through all drivers making sure they are
> > happy with using the adjusted_mode.crtc_ timings. I think most drivers
> > use the other adjusted_mode timings currently, and some even use the
> > user mode timings (which could be an actual bug).
>
> Let me take that back. What we want is to clip to the user mode
> timings. Stereo 3D needs the special treatment from
> drm_mode_get_hv_timing(). I'm getting confused by all these
> different timings we have all over the place.
>
> I think for i915 all we would need is to change the double wide/dual
> link adjustent for pipe_src_w to simply reject odd widths instead.
> That would guarantee that the user mode timings match the pipe_src_w/h
> 100%.
>
> For the other driver we'd need to figure out why they're using
> adjusted_mode timings for clipping. I guess that's just a mistake,
> which I repeated here with vmwgfx after getting confused by looking
> at the other drivers.
>
> I guess I just volunteered myself to do this. Just needs plenty of
> acks from driver maintainers I suppose.
Yeah consistently using drm_mode_get_hv_timing from
crtc_state->requested_mode seems like the right thing to do. Otherwise
there's a driver bug lurking somewhere ...
-Daniel
>
> >
> > >
> > > Either way, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >
> > > > ---
> > > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +++++++++++++----------
> > > > 1 file changed, 13 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > index 515b67783a41..efa41c086198 100644
> > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
> > > > int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > > > struct drm_plane_state *state)
> > > > {
> > > > + struct drm_crtc_state *crtc_state = NULL;
> > > > struct drm_framebuffer *new_fb = state->fb;
> > > > - struct drm_rect clip = {
> > > > - .x1 = state->crtc_x,
> > > > - .y1 = state->crtc_y,
> > > > - .x2 = state->crtc_x + state->crtc_w,
> > > > - .y2 = state->crtc_y + state->crtc_h,
> > > > - };
> > > > + struct drm_rect clip = {};
> > > > int ret;
> > > >
> > > > - ret = drm_plane_helper_check_state(state, &clip,
> > > > - DRM_PLANE_HELPER_NO_SCALING,
> > > > - DRM_PLANE_HELPER_NO_SCALING,
> > > > - false, true);
> > > > + if (state->crtc)
> > > > + crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
> > > >
> > > > + if (crtc_state && crtc_state->enable) {
> > > > + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > > + clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > > + }
> > > > +
> > > > + ret = drm_plane_helper_check_state(state, &clip,
> > > > + DRM_PLANE_HELPER_NO_SCALING,
> > > > + DRM_PLANE_HELPER_NO_SCALING,
> > > > + false, true);
> > > >
> > > > if (!ret && new_fb) {
> > > > struct drm_crtc *crtc = state->crtc;
> > > > --
> > > > 2.13.6
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
` (7 preceding siblings ...)
2017-11-01 21:52 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-11-10 21:26 ` Sinclair Yeh
2017-11-10 21:42 ` Ville Syrjälä
8 siblings, 1 reply; 31+ messages in thread
From: Sinclair Yeh @ 2017-11-10 21:26 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
Sorry this took so long.
The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
I've done some testing and the vmwgfx part looks good. Has Daniel
already taken these or should I put them in my next request?
Sinclair
On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> While trawling the tree I spotted some issues with the way vmwgfx
> uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> Do note that I haven't actually tested the resulting code at all,
> but it does build at least.
>
> And while touching that general area I took up Daniel's suggestion from
> long ago that drm_plane_helper_check_state() should be renamed and
> relocated to better reflect its status.
>
> Here's a branch with the entire series:
> git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
>
> Ville Syrjälä (5):
> drm/vmwgfx: Remove bogus crtc coords vs fb size check
> drm/vmwgfx: Use drm_plane_helper_check_state()
> drm/vmwgfx: Try to fix plane clipping
> drm: Check crtc_state->enable rather than crtc->enabled in
> drm_plane_helper_check_state()
> drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
>
> drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +-
> drivers/gpu/drm/arm/malidp_planes.c | 3 +-
> drivers/gpu/drm/drm_atomic_helper.c | 95 ++++++++++++++++++++++++
> drivers/gpu/drm/drm_plane_helper.c | 111 +++-------------------------
> drivers/gpu/drm/drm_simple_kms_helper.c | 9 ++-
> drivers/gpu/drm/i915/intel_display.c | 20 ++---
> drivers/gpu/drm/imx/ipuv3-plane.c | 8 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +-
> drivers/gpu/drm/meson/meson_plane.c | 8 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 18 +++--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
> drivers/gpu/drm/tegra/dc.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 ++++------
> drivers/gpu/drm/zte/zx_plane.c | 15 ++--
> include/drm/drm_atomic_helper.h | 7 ++
> include/drm/drm_plane_helper.h | 5 --
> 17 files changed, 187 insertions(+), 183 deletions(-)
>
> --
> 2.13.6
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
@ 2017-11-10 21:42 ` Ville Syrjälä
2017-11-20 7:34 ` Daniel Vetter
0 siblings, 1 reply; 31+ messages in thread
From: Ville Syrjälä @ 2017-11-10 21:42 UTC (permalink / raw)
To: Sinclair Yeh; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom, dri-devel
On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> Sorry this took so long.
No worries.
>
> The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
>
> I've done some testing and the vmwgfx part looks good. Has Daniel
> already taken these or should I put them in my next request?
You can take them, or I can push them to drm-misc-next. Whatever
works best for you.
And I'll want to revisit this topic soonish and move the clip
handling into the helper as discussed with Daniel. But that can
wait a bit until we get this round merged somewhere.
>
> Sinclair
>
> On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > While trawling the tree I spotted some issues with the way vmwgfx
> > uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> > Do note that I haven't actually tested the resulting code at all,
> > but it does build at least.
> >
> > And while touching that general area I took up Daniel's suggestion from
> > long ago that drm_plane_helper_check_state() should be renamed and
> > relocated to better reflect its status.
> >
> > Here's a branch with the entire series:
> > git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> >
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Sinclair Yeh <syeh@vmware.com>
> > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> >
> > Ville Syrjälä (5):
> > drm/vmwgfx: Remove bogus crtc coords vs fb size check
> > drm/vmwgfx: Use drm_plane_helper_check_state()
> > drm/vmwgfx: Try to fix plane clipping
> > drm: Check crtc_state->enable rather than crtc->enabled in
> > drm_plane_helper_check_state()
> > drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> >
> > drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +-
> > drivers/gpu/drm/arm/malidp_planes.c | 3 +-
> > drivers/gpu/drm/drm_atomic_helper.c | 95 ++++++++++++++++++++++++
> > drivers/gpu/drm/drm_plane_helper.c | 111 +++-------------------------
> > drivers/gpu/drm/drm_simple_kms_helper.c | 9 ++-
> > drivers/gpu/drm/i915/intel_display.c | 20 ++---
> > drivers/gpu/drm/imx/ipuv3-plane.c | 8 +-
> > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +-
> > drivers/gpu/drm/meson/meson_plane.c | 8 +-
> > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
> > drivers/gpu/drm/nouveau/nv50_display.c | 18 +++--
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
> > drivers/gpu/drm/tegra/dc.c | 4 +-
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 ++++------
> > drivers/gpu/drm/zte/zx_plane.c | 15 ++--
> > include/drm/drm_atomic_helper.h | 7 ++
> > include/drm/drm_plane_helper.h | 5 --
> > 17 files changed, 187 insertions(+), 183 deletions(-)
> >
> > --
> > 2.13.6
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-10 21:42 ` Ville Syrjälä
@ 2017-11-20 7:34 ` Daniel Vetter
2017-11-20 17:32 ` Sinclair Yeh
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2017-11-20 7:34 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Thomas Hellstrom, Sinclair Yeh, intel-gfx, dri-devel, VMware Graphics
On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > Sorry this took so long.
>
> No worries.
>
> >
> > The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> >
> > I've done some testing and the vmwgfx part looks good. Has Daniel
> > already taken these or should I put them in my next request?
>
> You can take them, or I can push them to drm-misc-next. Whatever
> works best for you.
>
> And I'll want to revisit this topic soonish and move the clip
> handling into the helper as discussed with Daniel. But that can
> wait a bit until we get this round merged somewhere.
Because we're still in the merge window I think it's probably best if we
push the entire series in through drm-misc. Tree-coordination in the merge
window is always a bit a pain.
-Daniel
>
> >
> > Sinclair
> >
> > On Wed, Nov 01, 2017 at 08:29:15PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > While trawling the tree I spotted some issues with the way vmwgfx
> > > uses drm_plane_helper_check_state(). Here's my attempt at fixing it.
> > > Do note that I haven't actually tested the resulting code at all,
> > > but it does build at least.
> > >
> > > And while touching that general area I took up Daniel's suggestion from
> > > long ago that drm_plane_helper_check_state() should be renamed and
> > > relocated to better reflect its status.
> > >
> > > Here's a branch with the entire series:
> > > git://github.com/vsyrjala/linux.git atomic_helper_plane_stuff
> > >
> > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > Cc: Sinclair Yeh <syeh@vmware.com>
> > > Cc: Thomas Hellstrom <thellstrom@vmware.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > >
> > > Ville Syrjälä (5):
> > > drm/vmwgfx: Remove bogus crtc coords vs fb size check
> > > drm/vmwgfx: Use drm_plane_helper_check_state()
> > > drm/vmwgfx: Try to fix plane clipping
> > > drm: Check crtc_state->enable rather than crtc->enabled in
> > > drm_plane_helper_check_state()
> > > drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
> > >
> > > drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +-
> > > drivers/gpu/drm/arm/malidp_planes.c | 3 +-
> > > drivers/gpu/drm/drm_atomic_helper.c | 95 ++++++++++++++++++++++++
> > > drivers/gpu/drm/drm_plane_helper.c | 111 +++-------------------------
> > > drivers/gpu/drm/drm_simple_kms_helper.c | 9 ++-
> > > drivers/gpu/drm/i915/intel_display.c | 20 ++---
> > > drivers/gpu/drm/imx/ipuv3-plane.c | 8 +-
> > > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +-
> > > drivers/gpu/drm/meson/meson_plane.c | 8 +-
> > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
> > > drivers/gpu/drm/nouveau/nv50_display.c | 18 +++--
> > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
> > > drivers/gpu/drm/tegra/dc.c | 4 +-
> > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 40 ++++------
> > > drivers/gpu/drm/zte/zx_plane.c | 15 ++--
> > > include/drm/drm_atomic_helper.h | 7 ++
> > > include/drm/drm_plane_helper.h | 5 --
> > > 17 files changed, 187 insertions(+), 183 deletions(-)
> > >
> > > --
> > > 2.13.6
> > >
>
> --
> Ville Syrjälä
> Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-20 7:34 ` Daniel Vetter
@ 2017-11-20 17:32 ` Sinclair Yeh
2017-11-20 19:36 ` Ville Syrjälä
0 siblings, 1 reply; 31+ messages in thread
From: Sinclair Yeh @ 2017-11-20 17:32 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, VMware Graphics, dri-devel, Thomas Hellstrom
On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > > Sorry this took so long.
> >
> > No worries.
> >
> > >
> > > The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > >
> > > I've done some testing and the vmwgfx part looks good. Has Daniel
> > > already taken these or should I put them in my next request?
> >
> > You can take them, or I can push them to drm-misc-next. Whatever
> > works best for you.
> >
> > And I'll want to revisit this topic soonish and move the clip
> > handling into the helper as discussed with Daniel. But that can
> > wait a bit until we get this round merged somewhere.
>
> Because we're still in the merge window I think it's probably best if we
> push the entire series in through drm-misc. Tree-coordination in the merge
> window is always a bit a pain.
> -Daniel
Ok, so I'll leave this series to you then.
thanks,
Sinclair
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-20 17:32 ` Sinclair Yeh
@ 2017-11-20 19:36 ` Ville Syrjälä
2017-11-23 9:56 ` Laurent Pinchart
0 siblings, 1 reply; 31+ messages in thread
From: Ville Syrjälä @ 2017-11-20 19:36 UTC (permalink / raw)
To: Sinclair Yeh; +Cc: intel-gfx, VMware Graphics, dri-devel, Thomas Hellstrom
On Mon, Nov 20, 2017 at 09:32:56AM -0800, Sinclair Yeh wrote:
> On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> > On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> > > On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> > > > Sorry this took so long.
> > >
> > > No worries.
> > >
> > > >
> > > > The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> > > >
> > > > I've done some testing and the vmwgfx part looks good. Has Daniel
> > > > already taken these or should I put them in my next request?
> > >
> > > You can take them, or I can push them to drm-misc-next. Whatever
> > > works best for you.
> > >
> > > And I'll want to revisit this topic soonish and move the clip
> > > handling into the helper as discussed with Daniel. But that can
> > > wait a bit until we get this round merged somewhere.
> >
> > Because we're still in the merge window I think it's probably best if we
> > push the entire series in through drm-misc. Tree-coordination in the merge
> > window is always a bit a pain.
> > -Daniel
>
> Ok, so I'll leave this series to you then.
All right. Series pushed to drm-misc-next. Thanks for the reviews.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping
2017-11-06 18:04 ` [Intel-gfx] " Ville Syrjälä
2017-11-07 12:30 ` Daniel Vetter
@ 2017-11-23 9:46 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:46 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
Hi Ville,
On Monday, 6 November 2017 20:04:38 EET Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> > On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> >> On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Try to fix the code to actually clip the plane to the crtc bounds
> >>> instead of the user provided crtc coordinates (which would be a no-op
> >>> since those are exactly the coordinates before clipping).
> >>>
> >>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> >>> Cc: Sinclair Yeh <syeh@vmware.com>
> >>> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> I kinda wonder whether we shouldn't push this down into the helper ...
I think that would be nice.
> > Would require quite going through all drivers making sure they are
> > happy with using the adjusted_mode.crtc_ timings. I think most drivers
> > use the other adjusted_mode timings currently, and some even use the
> > user mode timings (which could be an actual bug).
>
> Let me take that back. What we want is to clip to the user mode
> timings. Stereo 3D needs the special treatment from
> drm_mode_get_hv_timing(). I'm getting confused by all these
> different timings we have all over the place.
>
> I think for i915 all we would need is to change the double wide/dual
> link adjustent for pipe_src_w to simply reject odd widths instead.
> That would guarantee that the user mode timings match the pipe_src_w/h
> 100%.
>
> For the other driver we'd need to figure out why they're using
> adjusted_mode timings for clipping. I guess that's just a mistake,
> which I repeated here with vmwgfx after getting confused by looking
> at the other drivers.
I suppose it's a mix of cargo-cult and the fact that adjusted_mode hints that
it contains the timings that should be applied to the hardware. That's why I
use it in rcar-du.
Are drivers allowed to adjust the hdisplay and vdisplay values though ? I
thought unsupported values there had to be rejected with an error at atomic
check time, not adjusted.
> I guess I just volunteered myself to do this. Just needs plenty of
> acks from driver maintainers I suppose.
I'll review and test the rcar-du patch :-)
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
2017-11-01 20:15 ` [PATCH v2 " Ville Syrjala
@ 2017-11-23 9:52 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:52 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Hi Ville,
Thank you for the patch.
On Wednesday, 1 November 2017 20:29:19 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_plane_helper_check_state() is supposed to do things the atomic way,
> so it should not be inspecting crtc->enabled. Rather we should be
> looking at crtc_state->enable.
>
> We have a slight complication due to drm_plane_helper_check_update()
> reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
> we'll have to pass the crtc_state in manally and construct a fake
> crtc_state in drm_plane_helper_check_update().
The only user of drm_plane_helper_check_update() apart from vmwgfx which you
converted in patch 2/5 is the armada driver. It would be nice to convert it to
atomic modesetting and drop this function :-) At the very least, I'd add a
comment to drm_plane_helper_check_update() that this change should be reverted
when the function is dropped, otherwise we'll carry the CRTC state argument
for a long time without realizing it's not needed anymore.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +-
> drivers/gpu/drm/arm/malidp_planes.c | 3 +-
> drivers/gpu/drm/drm_plane_helper.c | 52 ++++++++++++++------------
> drivers/gpu/drm/drm_simple_kms_helper.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +-
> drivers/gpu/drm/meson/meson_plane.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +--
> drivers/gpu/drm/nouveau/nv50_display.c | 6 ++--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
> drivers/gpu/drm/tegra/dc.c | 4 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
> drivers/gpu/drm/zte/zx_plane.c | 4 +--
> include/drm/drm_plane_helper.h | 3 +-
> 15 files changed, 53 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> b/drivers/gpu/drm/arm/hdlcd_crtc.c index 72b22b805412..14721723fa8a 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c index 94e7e3fa3408..492d99dd55d4
> 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane
> *mp,
>
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c index 759ed93f4ba8..adb8d94484f2
> 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc,
>
> /**
> * drm_plane_helper_check_state() - Check plane state for validity
> - * @state: plane state to check
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> * @clip: integer clipping coordinates
> * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> @@ -120,35 +121,38 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc, * RETURNS:
> * Zero if update appears valid, error code on failure
> */
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> const struct drm_rect *clip,
> int min_scale,
> int max_scale,
> bool can_position,
> bool can_update_disabled)
> {
> - struct drm_crtc *crtc = state->crtc;
> - struct drm_framebuffer *fb = state->fb;
> - struct drm_rect *src = &state->src;
> - struct drm_rect *dst = &state->dst;
> - unsigned int rotation = state->rotation;
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_rect *src = &plane_state->src;
> + struct drm_rect *dst = &plane_state->dst;
> + unsigned int rotation = plane_state->rotation;
> int hscale, vscale;
>
> - *src = drm_plane_state_src(state);
> - *dst = drm_plane_state_dest(state);
> + WARN_ON(!crtc_state != !plane_state->crtc);
> + WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> +
> + *src = drm_plane_state_src(plane_state);
> + *dst = drm_plane_state_dest(plane_state);
>
> if (!fb) {
> - state->visible = false;
> + plane_state->visible = false;
> return 0;
> }
>
> /* crtc should only be NULL when disabling (i.e., !fb) */
> - if (WARN_ON(!crtc)) {
> - state->visible = false;
> + if (WARN_ON(!plane_state->crtc)) {
> + plane_state->visible = false;
> return 0;
> }
>
> - if (!crtc->enabled && !can_update_disabled) {
> + if (!crtc_state->enable && !can_update_disabled) {
> DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> return -EINVAL;
> }
> @@ -160,16 +164,16 @@ int drm_plane_helper_check_state(struct
> drm_plane_state *state, vscale = drm_rect_calc_vscale(src, dst, min_scale,
> max_scale);
> if (hscale < 0 || vscale < 0) {
> DRM_DEBUG_KMS("Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", &state->src, true);
> - drm_rect_debug_print("dst: ", &state->dst, false);
> + drm_rect_debug_print("src: ", &plane_state->src, true);
> + drm_rect_debug_print("dst: ", &plane_state->dst, false);
> return -ERANGE;
> }
>
> - state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> + plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale);
>
> drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
>
> - if (!state->visible)
> + if (!plane_state->visible)
> /*
> * Plane isn't visible; some drivers can handle this
> * so we just return success here. Drivers that can't
> @@ -230,7 +234,7 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, bool can_update_disabled,
> bool *visible)
> {
> - struct drm_plane_state state = {
> + struct drm_plane_state plane_state = {
> .plane = plane,
> .crtc = crtc,
> .fb = fb,
> @@ -245,18 +249,22 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, .rotation = rotation,
> .visible = *visible,
> };
> + struct drm_crtc_state crtc_state = {
> + .crtc = crtc,
> + .enable = crtc->enabled,
> + };
> int ret;
>
> - ret = drm_plane_helper_check_state(&state, clip,
> + ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> min_scale, max_scale,
> can_position,
> can_update_disabled);
> if (ret)
> return ret;
>
> - *src = state.src;
> - *dst = state.dst;
> - *visible = state.visible;
> + *src = plane_state.src;
> + *dst = plane_state.dst;
> + *visible = plane_state.visible;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c index dc9fd109de14..d428c805025c
> 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct
> drm_plane *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(plane_state, &clip,
> + ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 737de251d0f8..4a459726b36b
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state
> *crtc_state, int ret;
>
> ret = drm_plane_helper_check_state(&plane_state->base,
> + &crtc_state->base,
> &plane_state->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> @@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
> }
>
> ret = drm_plane_helper_check_state(&state->base,
> + &crtc_state->base,
> &state->clip,
> min_scale, max_scale,
> can_position, true);
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c index 247c60e6bed2..51b23ac175e3 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane
> *plane, clip.y1 = 0;
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> can_position, true);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 6f121891430f..7ebb33657704
> 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,7 +111,7 @@ static int mtk_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> diff --git a/drivers/gpu/drm/meson/meson_plane.c
> b/drivers/gpu/drm/meson/meson_plane.c index 17e96fa47868..2a00b720c371
> 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, &clip,
> + return drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index
> 4b22ac3413a1..1c194a9453d9 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct
> drm_crtc_state *crtc_state, min_scale = FRAC_16_16(1, 8);
> max_scale = FRAC_16_16(8, 1);
>
> - ret = drm_plane_helper_check_state(state, &clip, min_scale,
> - max_scale, true, true);
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> + min_scale, max_scale, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c index e4751f92b342..7f11a88d0593
> 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, {
> int ret;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> + ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> true, true);
> @@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, if (!fb->format->depth)
> return -EINVAL;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
> + ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index
> 19128b4dea54..36d0f101e30d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> min_scale, max_scale,
> true, true);
> if (ret)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 24a5ef4f5bb8..e95d9e2a7f6c 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane
> *plane, clip.y2 = crtc_state->mode.vdisplay;
>
> /* Check plane state for visibility and calculate clipping bounds */
> - err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
> - true, true);
> + err = drm_plane_helper_check_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (err < 0)
> return err;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index efa41c086198..d95e7b1c3b11
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, clip.y2 = crtc_state->adjusted_mode.vdisplay;
> }
>
> - ret = drm_plane_helper_check_state(state, &clip,
> + ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index 18e763493264..ee0002529b8f 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, &clip,
> + return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> min_scale, max_scale,
> true, true);
> }
> @@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, &clip,
> + return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> false, true);
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 7c8a00ceadb7..41b8309b0a57 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,7 +38,8 @@
> */
> #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>
> -int drm_plane_helper_check_state(struct drm_plane_state *state,
> +int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> + const struct drm_crtc_state *crtc_state,
> const struct drm_rect *clip,
> int min_scale, int max_scale,
> bool can_position,
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
2017-11-01 20:16 ` [PATCH v2 " Ville Syrjala
@ 2017-11-23 9:53 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:53 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Hi Ville,
Thank you for the patch.
On Wednesday, 1 November 2017 20:29:20 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_plane_helper_check_update() isn't a transitional helper, so let's
> rename it to drm_atomic_helper_check_plane_state() and move it into
> drm_atomic_helper.c.
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I would move this patch before 4/5 to make it easier to revert 4/5 (for the
reason explaining as a reply to that patch).
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +--
> drivers/gpu/drm/arm/malidp_planes.c | 4 +-
> drivers/gpu/drm/drm_atomic_helper.c | 95 +++++++++++++++++++++++++
> drivers/gpu/drm/drm_plane_helper.c | 103 +-----------------------
> drivers/gpu/drm/drm_simple_kms_helper.c | 9 +--
> drivers/gpu/drm/i915/intel_display.c | 22 +++---
> drivers/gpu/drm/imx/ipuv3-plane.c | 8 +--
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 8 +--
> drivers/gpu/drm/meson/meson_plane.c | 8 +--
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 5 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 20 +++---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +-
> drivers/gpu/drm/tegra/dc.c | 4 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 8 +--
> drivers/gpu/drm/zte/zx_plane.c | 15 ++--
> include/drm/drm_atomic_helper.h | 7 ++
> include/drm/drm_plane_helper.h | 6 --
> 17 files changed, 170 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> b/drivers/gpu/drm/arm/hdlcd_crtc.c index 14721723fa8a..63511a3bbf6c 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -252,10 +252,10 @@ static int hdlcd_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> }
>
> static void hdlcd_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c index 492d99dd55d4..72a07950167e
> 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -150,8 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane
> *mp,
>
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - 0, INT_MAX, true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 71d712f1b56a..7595ad8ad2f3
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -696,6 +696,101 @@ drm_atomic_helper_check_modeset(struct drm_device
> *dev, EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>
> /**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: crtc state to check
> + * @clip: integer clipping coordinates
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + * doesn't cover the entire crtc? This will generally
> + * only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the crtc
> + * is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state
> *plane_state, + const struct drm_crtc_state *crtc_state,
> + const struct drm_rect *clip,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled)
> +{
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct drm_rect *src = &plane_state->src;
> + struct drm_rect *dst = &plane_state->dst;
> + unsigned int rotation = plane_state->rotation;
> + int hscale, vscale;
> +
> + WARN_ON(!crtc_state != !plane_state->crtc);
> + WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> +
> + *src = drm_plane_state_src(plane_state);
> + *dst = drm_plane_state_dest(plane_state);
> +
> + if (!fb) {
> + plane_state->visible = false;
> + return 0;
> + }
> +
> + /* crtc should only be NULL when disabling (i.e., !fb) */
> + if (WARN_ON(!plane_state->crtc)) {
> + plane_state->visible = false;
> + return 0;
> + }
> +
> + if (!crtc_state->enable && !can_update_disabled) {
> + DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> + return -EINVAL;
> + }
> +
> + drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> +
> + /* Check scaling */
> + hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> + vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> + if (hscale < 0 || vscale < 0) {
> + DRM_DEBUG_KMS("Invalid scaling of plane\n");
> + drm_rect_debug_print("src: ", &plane_state->src, true);
> + drm_rect_debug_print("dst: ", &plane_state->dst, false);
> + return -ERANGE;
> + }
> +
> + plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale); +
> + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> +
> + if (!plane_state->visible)
> + /*
> + * Plane isn't visible; some drivers can handle this
> + * so we just return success here. Drivers that can't
> + * (including those that use the primary plane helper's
> + * update function) will return an error from their
> + * update_plane handler.
> + */
> + return 0;
> +
> + if (!can_position && !drm_rect_equals(dst, clip)) {
> + DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> + drm_rect_debug_print("dst: ", dst, false);
> + drm_rect_debug_print("clip: ", clip, false);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> +
> +/**
> * drm_atomic_helper_check_planes - validate state object for planes
> changes * @dev: DRM device
> * @state: the driver state object
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c index adb8d94484f2..f1be8cd4e387
> 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -100,101 +100,6 @@ static int get_connectors_for_crtc(struct drm_crtc
> *crtc, }
>
> /**
> - * drm_plane_helper_check_state() - Check plane state for validity
> - * @plane_state: plane state to check
> - * @crtc_state: crtc state to check
> - * @clip: integer clipping coordinates
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> - * @can_position: is it legal to position the plane such that it
> - * doesn't cover the entire crtc? This will generally
> - * only be false for primary planes.
> - * @can_update_disabled: can the plane be updated while the crtc
> - * is disabled?
> - *
> - * Checks that a desired plane update is valid, and updates various
> - * bits of derived state (clipped coordinates etc.). Drivers that provide
> - * their own plane handling rather than helper-provided implementations may
> - * still wish to call this function to avoid duplication of error checking
> - * code.
> - *
> - * RETURNS:
> - * Zero if update appears valid, error code on failure
> - */
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> - const struct drm_crtc_state *crtc_state,
> - const struct drm_rect *clip,
> - int min_scale,
> - int max_scale,
> - bool can_position,
> - bool can_update_disabled)
> -{
> - struct drm_framebuffer *fb = plane_state->fb;
> - struct drm_rect *src = &plane_state->src;
> - struct drm_rect *dst = &plane_state->dst;
> - unsigned int rotation = plane_state->rotation;
> - int hscale, vscale;
> -
> - WARN_ON(!crtc_state != !plane_state->crtc);
> - WARN_ON(crtc_state && crtc_state->crtc != plane_state->crtc);
> -
> - *src = drm_plane_state_src(plane_state);
> - *dst = drm_plane_state_dest(plane_state);
> -
> - if (!fb) {
> - plane_state->visible = false;
> - return 0;
> - }
> -
> - /* crtc should only be NULL when disabling (i.e., !fb) */
> - if (WARN_ON(!plane_state->crtc)) {
> - plane_state->visible = false;
> - return 0;
> - }
> -
> - if (!crtc_state->enable && !can_update_disabled) {
> - DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
> - return -EINVAL;
> - }
> -
> - drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> -
> - /* Check scaling */
> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> - if (hscale < 0 || vscale < 0) {
> - DRM_DEBUG_KMS("Invalid scaling of plane\n");
> - drm_rect_debug_print("src: ", &plane_state->src, true);
> - drm_rect_debug_print("dst: ", &plane_state->dst, false);
> - return -ERANGE;
> - }
> -
> - plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale,
> vscale); -
> - drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> -
> - if (!plane_state->visible)
> - /*
> - * Plane isn't visible; some drivers can handle this
> - * so we just return success here. Drivers that can't
> - * (including those that use the primary plane helper's
> - * update function) will return an error from their
> - * update_plane handler.
> - */
> - return 0;
> -
> - if (!can_position && !drm_rect_equals(dst, clip)) {
> - DRM_DEBUG_KMS("Plane must cover entire CRTC\n");
> - drm_rect_debug_print("dst: ", dst, false);
> - drm_rect_debug_print("clip: ", clip, false);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_plane_helper_check_state);
> -
> -/**
> * drm_plane_helper_check_update() - Check plane update for validity
> * @plane: plane object to update
> * @crtc: owning CRTC of owning plane
> @@ -255,10 +160,10 @@ int drm_plane_helper_check_update(struct drm_plane
> *plane, };
> int ret;
>
> - ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
> - min_scale, max_scale,
> - can_position,
> - can_update_disabled);
> + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> + clip, min_scale, max_scale,
> + can_position,
> + can_update_disabled);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c index d428c805025c..9f3b1c94802b
> 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -103,10 +103,11 @@ static int drm_simple_kms_plane_atomic_check(struct
> drm_plane *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 4a459726b36b..e0ff599d2090
> 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9316,12 +9316,12 @@ static int intel_check_cursor(struct
> intel_crtc_state *crtc_state, u32 offset;
> int ret;
>
> - ret = drm_plane_helper_check_state(&plane_state->base,
> - &crtc_state->base,
> - &plane_state->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(&plane_state->base,
> + &crtc_state->base,
> + &plane_state->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> if (ret)
> return ret;
>
> @@ -12808,11 +12808,11 @@ intel_check_primary_plane(struct intel_plane
> *plane, can_position = true;
> }
>
> - ret = drm_plane_helper_check_state(&state->base,
> - &crtc_state->base,
> - &state->clip,
> - min_scale, max_scale,
> - can_position, true);
> + ret = drm_atomic_helper_check_plane_state(&state->base,
> + &crtc_state->base,
> + &state->clip,
> + min_scale, max_scale,
> + can_position, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c
> b/drivers/gpu/drm/imx/ipuv3-plane.c index 51b23ac175e3..5a67daedcf4d 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -342,10 +342,10 @@ static int ipu_plane_atomic_check(struct drm_plane
> *plane, clip.y1 = 0;
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - can_position, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + can_position, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 7ebb33657704..5ef898b93d8d
> 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -111,10 +111,10 @@ static int mtk_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> }
>
> static void mtk_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/meson/meson_plane.c
> b/drivers/gpu/drm/meson/meson_plane.c index 2a00b720c371..d0a6ac8390f3
> 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -61,10 +61,10 @@ static int meson_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->mode.hdisplay;
> clip.y2 = crtc_state->mode.vdisplay;
>
> - return drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + return drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> }
>
> /* Takes a fixed 16.16 number and converts it to integer. */
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index
> 1c194a9453d9..df3360acacca 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -348,8 +348,9 @@ static int mdp5_plane_atomic_check_with_state(struct
> drm_crtc_state *crtc_state, min_scale = FRAC_16_16(1, 8);
> max_scale = FRAC_16_16(8, 1);
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - min_scale, max_scale, true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + min_scale, max_scale,
> + true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c index 7f11a88d0593..8ab0fac3544d
> 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1143,11 +1143,11 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, {
> int ret;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> - &asyw->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + true, true);
> asyh->curs.visible = asyw->state.visible;
> if (ret || !asyh->curs.visible)
> return ret;
> @@ -1433,11 +1433,11 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw, if (!fb->format->depth)
> return -EINVAL;
>
> - ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
> - &asyw->clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state,
> + &asyw->clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index
> 36d0f101e30d..ba7505292b78 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -659,9 +659,9 @@ static int vop_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - min_scale, max_scale,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + min_scale, max_scale,
> + true, true);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index e95d9e2a7f6c..fc70351b9017 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane
> *plane, clip.y2 = crtc_state->mode.vdisplay;
>
> /* Check plane state for visibility and calculate clipping bounds */
> - err = drm_plane_helper_check_state(state, crtc_state, &clip,
> - 0, INT_MAX, true, true);
> + err = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + 0, INT_MAX, true, true);
> if (err < 0)
> return err;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index d95e7b1c3b11..a2a93d7e2a04
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -454,10 +454,10 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, clip.y2 = crtc_state->adjusted_mode.vdisplay;
> }
>
> - ret = drm_plane_helper_check_state(state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
>
> if (!ret && new_fb) {
> struct drm_crtc *crtc = state->crtc;
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index ee0002529b8f..68fd2e2dc78a 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -80,9 +80,9 @@ static int zx_vl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - min_scale, max_scale,
> - true, true);
> + return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip, min_scale, max_scale,
> + true, true);
> }
>
> static int zx_vl_get_fmt(uint32_t format)
> @@ -315,10 +315,11 @@ static int zx_gl_plane_atomic_check(struct drm_plane
> *plane, clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> - return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
> - DRM_PLANE_HELPER_NO_SCALING,
> - DRM_PLANE_HELPER_NO_SCALING,
> - false, true);
> + return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
> + &clip,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, true);
> }
>
> static int zx_gl_get_fmt(uint32_t format)
> diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h index d2b56cc657e9..4842ee9485ce 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -38,6 +38,13 @@ struct drm_private_state;
>
> int drm_atomic_helper_check_modeset(struct drm_device *dev,
> struct drm_atomic_state *state);
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state
> *plane_state, + const struct drm_crtc_state *crtc_state,
> + const struct drm_rect *clip,
> + int min_scale,
> + int max_scale,
> + bool can_position,
> + bool can_update_disabled);
> int drm_atomic_helper_check_planes(struct drm_device *dev,
> struct drm_atomic_state *state);
> int drm_atomic_helper_check(struct drm_device *dev,
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 41b8309b0a57..8aa49c0ecd4d 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -38,12 +38,6 @@
> */
> #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
>
> -int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
> - const struct drm_crtc_state *crtc_state,
> - const struct drm_rect *clip,
> - int min_scale, int max_scale,
> - bool can_position,
> - bool can_update_disabled);
> int drm_plane_helper_check_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
2017-11-02 10:04 ` Daniel Vetter
@ 2017-11-23 9:54 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:54 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
Hi Ville,
Thank you for the patch.
On Wednesday, 1 November 2017 20:29:16 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Throw away the bugs crtc coords vs. fb size check. Crtc coords don't
> define the viewport inside the fb, that's a job for the src coords,
> which have been checked by the core already.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 0545740b3724..a4b56699679a
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -476,12 +476,6 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane,
>
> vcs = vmw_connector_state_to_vcs(du->connector.state);
>
> - if ((dest.x2 > new_fb->width ||
> - dest.y2 > new_fb->height)) {
> - DRM_ERROR("CRTC area outside of framebuffer\n");
> - return -EINVAL;
> - }
> -
> /* Only one active implicit framebuffer at a time. */
> mutex_lock(&dev_priv->global_kms_state_mutex);
> if (vcs->is_implicit && dev_priv->implicit_fb &&
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state()
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
2017-11-02 10:06 ` Daniel Vetter
@ 2017-11-23 9:54 ` Laurent Pinchart
1 sibling, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:54 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
Hi Ville,
Thank you for the patch.
On Wednesday, 1 November 2017 20:29:17 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Atomic drivers have no reason to use drm_plane_helper_check_update()
> instead of drm_plane_helper_check_state(). So let's switch over.
>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index a4b56699679a..515b67783a41
> 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -442,29 +442,18 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane
> *plane, struct drm_plane_state *state)
> {
> struct drm_framebuffer *new_fb = state->fb;
> - bool visible;
> -
> - struct drm_rect src = {
> - .x1 = state->src_x,
> - .y1 = state->src_y,
> - .x2 = state->src_x + state->src_w,
> - .y2 = state->src_y + state->src_h,
> - };
> - struct drm_rect dest = {
> + struct drm_rect clip = {
> .x1 = state->crtc_x,
> .y1 = state->crtc_y,
> .x2 = state->crtc_x + state->crtc_w,
> .y2 = state->crtc_y + state->crtc_h,
> };
> - struct drm_rect clip = dest;
> int ret;
>
> - ret = drm_plane_helper_check_update(plane, state->crtc, new_fb,
> - &src, &dest, &clip,
> - DRM_MODE_ROTATE_0,
> + ret = drm_plane_helper_check_state(state, &clip,
> DRM_PLANE_HELPER_NO_SCALING,
> DRM_PLANE_HELPER_NO_SCALING,
> - false, true, &visible);
> + false, true);
>
>
> if (!ret && new_fb) {
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff
2017-11-20 19:36 ` Ville Syrjälä
@ 2017-11-23 9:56 ` Laurent Pinchart
0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2017-11-23 9:56 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, VMware Graphics, Thomas Hellstrom
Hi Ville,
On Monday, 20 November 2017 21:36:46 EET Ville Syrjälä wrote:
> On Mon, Nov 20, 2017 at 09:32:56AM -0800, Sinclair Yeh wrote:
> > On Mon, Nov 20, 2017 at 08:34:50AM +0100, Daniel Vetter wrote:
> >> On Fri, Nov 10, 2017 at 11:42:59PM +0200, Ville Syrjälä wrote:
> >>> On Fri, Nov 10, 2017 at 01:26:47PM -0800, Sinclair Yeh wrote:
> >>> > Sorry this took so long.
> >>>
> >>> No worries.
> >>>
> >>>> The vmwgfx part: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
> >>>>
> >>>> I've done some testing and the vmwgfx part looks good. Has Daniel
> >>>> already taken these or should I put them in my next request?
> >>>
> >>> You can take them, or I can push them to drm-misc-next. Whatever
> >>> works best for you.
> >>>
> >>> And I'll want to revisit this topic soonish and move the clip
> >>> handling into the helper as discussed with Daniel. But that can
> >>> wait a bit until we get this round merged somewhere.
> >>
> >> Because we're still in the merge window I think it's probably best if we
> >> push the entire series in through drm-misc. Tree-coordination in the
> >> merge window is always a bit a pain.
> >
> > Ok, so I'll leave this series to you then.
>
> All right. Series pushed to drm-misc-next. Thanks for the reviews.
And I now realize my review comments aren't very useful (apart possibly the
one about adding a comment to the drm_plane_helper_check_update() function,
feel free to submit a patch for that if you think it's useful) as the series
has been merged already :-/ I should really start reading mails in the reverse
chronological order.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2017-11-23 9:57 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
2017-11-02 10:04 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
2017-11-02 10:06 ` Daniel Vetter
2017-11-23 9:54 ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
2017-11-02 10:12 ` Daniel Vetter
2017-11-02 13:19 ` Ville Syrjälä
2017-11-06 18:04 ` [Intel-gfx] " Ville Syrjälä
2017-11-07 12:30 ` Daniel Vetter
2017-11-23 9:46 ` [Intel-gfx] " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
2017-11-01 20:15 ` [PATCH v2 " Ville Syrjala
2017-11-02 10:15 ` Daniel Vetter
2017-11-23 9:52 ` [PATCH " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
2017-11-01 20:16 ` [PATCH v2 " Ville Syrjala
2017-11-02 10:16 ` Daniel Vetter
2017-11-23 9:53 ` [PATCH " Laurent Pinchart
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
2017-11-01 20:10 ` Ville Syrjälä
2017-11-01 21:02 ` ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3) Patchwork
2017-11-01 21:52 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
2017-11-10 21:42 ` Ville Syrjälä
2017-11-20 7:34 ` Daniel Vetter
2017-11-20 17:32 ` Sinclair Yeh
2017-11-20 19:36 ` Ville Syrjälä
2017-11-23 9:56 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.