From: Ville Syrjälä ville.syrjala@linux.intel.com
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c8422..66233a8 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2342,7 +2342,8 @@ static int __setplane_internal(struct drm_plane *plane, crtc_y > INT_MAX - (int32_t) crtc_h) { DRM_DEBUG_KMS("Invalid CRTC coordinates %ux%u+%d+%d\n", crtc_w, crtc_h, crtc_x, crtc_y); - return -ERANGE; + ret = -ERANGE; + goto out; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
When converting the mode hdisplay/vdisplay to primary plane src coordinates we need to take into account the current plane rotation.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_atomic_helper.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 87a2a44..0c6f621 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1790,8 +1790,13 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, primary_state->crtc_w = set->mode->hdisplay; primary_state->src_x = set->x << 16; primary_state->src_y = set->y << 16; - primary_state->src_h = set->mode->vdisplay << 16; - primary_state->src_w = set->mode->hdisplay << 16; + if (primary_state->rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) { + primary_state->src_h = set->mode->hdisplay << 16; + primary_state->src_w = set->mode->vdisplay << 16; + } else { + primary_state->src_h = set->mode->vdisplay << 16; + primary_state->src_w = set->mode->hdisplay << 16; + }
commit: ret = update_output_state(state, set);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Pull the plane src coordinate checks into a separate function so that we can share them for the legacy and new stuff.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 59 +++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 66233a8..227613a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2286,6 +2286,32 @@ int drm_plane_check_pixel_format(const struct drm_plane *plane, u32 format) return -EINVAL; }
+static int check_src_coords(uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h, + const struct drm_framebuffer *fb) +{ + unsigned int fb_width, fb_height; + + fb_width = fb->width << 16; + fb_height = fb->height << 16; + + /* Make sure source coordinates are inside the fb. */ + if (src_w > fb_width || + src_x > fb_width - src_w || + src_h > fb_height || + src_y > fb_height - src_h) { + DRM_DEBUG_KMS("Invalid source coordinates " + "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n", + src_w >> 16, ((src_w & 0xffff) * 15625) >> 10, + src_h >> 16, ((src_h & 0xffff) * 15625) >> 10, + src_x >> 16, ((src_x & 0xffff) * 15625) >> 10, + src_y >> 16, ((src_y & 0xffff) * 15625) >> 10); + return -ENOSPC; + } + + return 0; +} + /* * setplane_internal - setplane handler for internal callers * @@ -2305,7 +2331,6 @@ static int __setplane_internal(struct drm_plane *plane, uint32_t src_w, uint32_t src_h) { int ret = 0; - unsigned int fb_width, fb_height;
/* No fb means shut it down */ if (!fb) { @@ -2346,24 +2371,9 @@ static int __setplane_internal(struct drm_plane *plane, goto out; }
- - fb_width = fb->width << 16; - fb_height = fb->height << 16; - - /* Make sure source coordinates are inside the fb. */ - if (src_w > fb_width || - src_x > fb_width - src_w || - src_h > fb_height || - src_y > fb_height - src_h) { - DRM_DEBUG_KMS("Invalid source coordinates " - "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n", - src_w >> 16, ((src_w & 0xffff) * 15625) >> 10, - src_h >> 16, ((src_h & 0xffff) * 15625) >> 10, - src_x >> 16, ((src_x & 0xffff) * 15625) >> 10, - src_y >> 16, ((src_y & 0xffff) * 15625) >> 10); - ret = -ENOSPC; + ret = check_src_coords(src_x, src_y, src_w, src_h, fb); + if (ret) goto out; - }
plane->old_fb = plane->fb; ret = plane->funcs->update_plane(plane, crtc, fb, @@ -2557,17 +2567,8 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc, if (crtc->invert_dimensions) swap(hdisplay, vdisplay);
- if (hdisplay > fb->width || - vdisplay > fb->height || - x > fb->width - hdisplay || - y > fb->height - vdisplay) { - DRM_DEBUG_KMS("Invalid fb size %ux%u for CRTC viewport %ux%u+%d+%d%s.\n", - fb->width, fb->height, hdisplay, vdisplay, x, y, - crtc->invert_dimensions ? " (inverted)" : ""); - return -ENOSPC; - } - - return 0; + return check_src_coords(x << 16, y << 16, + hdisplay << 16, vdisplay << 16, fb); } EXPORT_SYMBOL(drm_crtc_check_viewport);
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{ + if (crtc->state) { + return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) | + BIT(DRM_ROTATE_270)); + } else { + return crtc->invert_dimensions; + } +} + /** * drm_crtc_check_viewport - Checks that a framebuffer is big enough for the * CRTC viewport @@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions) + if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
On Thu, Oct 15, 2015 at 08:40:01PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Can't we just remove the invert_dimensions field completely now? It's a legacy-only field, but no legacy drivers actually set it, so it winds up being completely unused.
Matt
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{
- if (crtc->state) {
return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |BIT(DRM_ROTATE_270));- } else {
return crtc->invert_dimensions;- }
+}
/**
- drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
CRTC viewport@@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions)
if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
-- 2.4.9
On Thu, Oct 15, 2015 at 05:32:12PM -0700, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:01PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Can't we just remove the invert_dimensions field completely now? It's a legacy-only field, but no legacy drivers actually set it, so it winds up being completely unused.
omap sets it.
Matt
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{
- if (crtc->state) {
return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |BIT(DRM_ROTATE_270));- } else {
return crtc->invert_dimensions;- }
+}
/**
- drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
CRTC viewport@@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions)
if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
-- 2.4.9
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
On Fri, Oct 16, 2015 at 11:38:18AM +0300, Ville Syrjälä wrote:
On Thu, Oct 15, 2015 at 05:32:12PM -0700, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:01PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Can't we just remove the invert_dimensions field completely now? It's a legacy-only field, but no legacy drivers actually set it, so it winds up being completely unused.
omap sets it.
Omap should be atomic now (maybe double-check with Laurent), so I think we can indeed ditch it. Follow-up series perhaps? -Daniel
Matt
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{
- if (crtc->state) {
return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |BIT(DRM_ROTATE_270));- } else {
return crtc->invert_dimensions;- }
+}
/**
- drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
CRTC viewport@@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions)
if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
-- 2.4.9
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
-- Ville Syrjälä Intel OTC
On Fri, Oct 16, 2015 at 04:35:02PM +0200, Daniel Vetter wrote:
On Fri, Oct 16, 2015 at 11:38:18AM +0300, Ville Syrjälä wrote:
On Thu, Oct 15, 2015 at 05:32:12PM -0700, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:01PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Can't we just remove the invert_dimensions field completely now? It's a legacy-only field, but no legacy drivers actually set it, so it winds up being completely unused.
omap sets it.
Omap should be atomic now (maybe double-check with Laurent), so I think we can indeed ditch it. Follow-up series perhaps?
grep ATOMIC in omap didn't turn up anything, so I assumed no.
-Daniel
Matt
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{
- if (crtc->state) {
return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |BIT(DRM_ROTATE_270));- } else {
return crtc->invert_dimensions;- }
+}
/**
- drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
CRTC viewport@@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions)
if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
-- 2.4.9
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
-- Ville Syrjälä Intel OTC
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Fri, Oct 16, 2015 at 06:10:20PM +0300, Ville Syrjälä wrote:
On Fri, Oct 16, 2015 at 04:35:02PM +0200, Daniel Vetter wrote:
On Fri, Oct 16, 2015 at 11:38:18AM +0300, Ville Syrjälä wrote:
On Thu, Oct 15, 2015 at 05:32:12PM -0700, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:01PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
Can't we just remove the invert_dimensions field completely now? It's a legacy-only field, but no legacy drivers actually set it, so it winds up being completely unused.
omap sets it.
Omap should be atomic now (maybe double-check with Laurent), so I think we can indeed ditch it. Follow-up series perhaps?
grep ATOMIC in omap didn't turn up anything, so I assumed no.
omap might not be full atomic (just like i915 isn't full atomic), but it's state-based now, so your new code below will cause the invert_dimensions to be ignored by the DRM core. And omap never uses the field internally, aside from setting it once to reflect the state values, so as far as I can see, the field is completely unused.
Matt
-Daniel
Matt
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_crtc.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..ffaa3f5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2545,6 +2545,16 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+static bool invert_dimensions(const struct drm_crtc *crtc) +{
- if (crtc->state) {
return crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |BIT(DRM_ROTATE_270));- } else {
return crtc->invert_dimensions;- }
+}
/**
- drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
CRTC viewport@@ -2564,7 +2574,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions)
if (invert_dimensions(crtc)) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16,
-- 2.4.9
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
-- Ville Syrjälä Intel OTC
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- Ville Syrjälä Intel OTC
From: Ville Syrjälä ville.syrjala@linux.intel.com
On atomic drivers we can dig out the primary plane rotation from the plane state instead of looking at the legacy crtc->invert_dimensions flag. The flag is not set by anyone except omapdrm, and it would be racy to set it the same way in the atomic helpers.
v2: Kill crtc->invert_dimensions totally since omap is state based already and no one else ever used it (Matt)
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Cc: Tomi Valkeinen tomi.valkeinen@ti.com Cc: Rob Clark robdclark@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 5 +++-- drivers/gpu/drm/omapdrm/omap_crtc.c | 3 --- include/drm/drm_crtc.h | 5 ----- 3 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 227613a..a7738be 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -677,7 +677,6 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
crtc->dev = dev; crtc->funcs = funcs; - crtc->invert_dimensions = false;
drm_modeset_lock_init(&crtc->mutex); ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC); @@ -2564,7 +2563,9 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- if (crtc->invert_dimensions) + if (crtc->state && + crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) | + BIT(DRM_ROTATE_270))) swap(hdisplay, vdisplay);
return check_src_coords(x << 16, y << 16, diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 9a4ba4f..ad09590 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -412,9 +412,6 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc, dispc_mgr_go(omap_crtc->channel); omap_irq_register(crtc->dev, &omap_crtc->vblank_irq); } - - crtc->invert_dimensions = !!(crtc->primary->state->rotation & - (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))); }
static int omap_crtc_atomic_set_property(struct drm_crtc *crtc, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 317baf9..028e5d4 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -407,9 +407,6 @@ struct drm_crtc_funcs { * @enabled: is this CRTC enabled? * @mode: current mode timings * @hwmode: mode timings as programmed to hw regs - * @invert_dimensions: for purposes of error checking crtc vs fb sizes, - * invert the width/height of the crtc. This is used if the driver - * is performing 90 or 270 degree rotated scanout * @x: x position on screen * @y: y position on screen * @funcs: CRTC control functions @@ -458,8 +455,6 @@ struct drm_crtc { */ struct drm_display_mode hwmode;
- bool invert_dimensions; - int x, y; const struct drm_crtc_funcs *funcs;
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary plane coordinates from the plane state when checking them against the new framebuffer during page flip.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ffaa3f5..52feffc 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5193,7 +5193,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; }
- ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); + if (crtc->state) { + const struct drm_plane_state *state = crtc->primary->state; + + ret = check_src_coords(state->src_x, state->src_y, + state->src_w, state->src_h, fb); + } else { + ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb); + } if (ret) goto out;
On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary plane coordinates from the plane state when checking them against the new framebuffer during page flip.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
For the series:
Reviewed-by: Matt Roper matthew.d.roper@intel.com
I also confirmed that the i-g-t test I wrote here: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html now passes with your patch series, so I believe Tvrtko's original bug report should be fixed.
Matt
drivers/gpu/drm/drm_crtc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ffaa3f5..52feffc 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5193,7 +5193,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; }
- ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);
- if (crtc->state) {
const struct drm_plane_state *state = crtc->primary->state;ret = check_src_coords(state->src_x, state->src_y,state->src_w, state->src_h, fb);- } else {
ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);- } if (ret) goto out;
-- 2.4.9
On 16/10/15 17:27, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary plane coordinates from the plane state when checking them against the new framebuffer during page flip.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
For the series:
Reviewed-by: Matt Roper matthew.d.roper@intel.com
I also confirmed that the i-g-t test I wrote here: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html now passes with your patch series, so I believe Tvrtko's original bug report should be fixed.
Oh I did not realize this series is about this, perhaps because I did not see 1/5 which maybe had some more obvious clues. :)
Great, that means if we decide to merge "drm/i915: Consider plane rotation when calculating stride in skl_do_mmio_flip" and "kms_rotation_crc: Exercise page flips with 90 degree rotation" we would have working rotated legacy page flip with a test case.
Regards,
Tvrtko
On Fri, Oct 16, 2015 at 05:40:59PM +0100, Tvrtko Ursulin wrote:
On 16/10/15 17:27, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary plane coordinates from the plane state when checking them against the new framebuffer during page flip.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
For the series:
Reviewed-by: Matt Roper matthew.d.roper@intel.com
Pulled all 5 into drm-misc, thanks.
I also confirmed that the i-g-t test I wrote here: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html now passes with your patch series, so I believe Tvrtko's original bug report should be fixed.
Oh I did not realize this series is about this, perhaps because I did not see 1/5 which maybe had some more obvious clues. :)
Great, that means if we decide to merge "drm/i915: Consider plane rotation when calculating stride in skl_do_mmio_flip" and "kms_rotation_crc: Exercise page flips with 90 degree rotation" we would have working rotated legacy page flip with a test case.
Matt, can you please push these igt patches and double-check that we have subtests to catch the viewport confusion here?
Thanks, Daniel
On Fri, Oct 16, 2015 at 07:17:31PM +0200, Daniel Vetter wrote:
On Fri, Oct 16, 2015 at 05:40:59PM +0100, Tvrtko Ursulin wrote:
On 16/10/15 17:27, Matt Roper wrote:
On Thu, Oct 15, 2015 at 08:40:02PM +0300, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Instead of relying on the old crtc-{x,y,mode} gunk, dig out the primary plane coordinates from the plane state when checking them against the new framebuffer during page flip.
Cc: Matt Roper matthew.d.roper@intel.com Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
For the series:
Reviewed-by: Matt Roper matthew.d.roper@intel.com
Pulled all 5 into drm-misc, thanks.
I also confirmed that the i-g-t test I wrote here: http://lists.freedesktop.org/archives/intel-gfx/2015-October/077394.html now passes with your patch series, so I believe Tvrtko's original bug report should be fixed.
Oh I did not realize this series is about this, perhaps because I did not see 1/5 which maybe had some more obvious clues. :)
Great, that means if we decide to merge "drm/i915: Consider plane rotation when calculating stride in skl_do_mmio_flip" and "kms_rotation_crc: Exercise page flips with 90 degree rotation" we would have working rotated legacy page flip with a test case.
Matt, can you please push these igt patches and double-check that we have subtests to catch the viewport confusion here?
Thanks, Daniel
Looks like Thomas already beat me to pushing the relevant i-g-t patches (both the one I wrote and the one Tvrtko wrote), so I think we're covered there.
Thanks.
Matt
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Matt Roper -
Tvrtko Ursulin -
Ville Syrjälä -
ville.syrjala＠linux.intel.com