* [PATCH] drm/i915/skl: don't fail colorkey + scaler request
@ 2015-05-18 23:18 Chandra Konduru
2015-05-19 23:47 ` shuang.he
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chandra Konduru @ 2015-05-18 23:18 UTC (permalink / raw)
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala, bin.a.xu
There is a mplayer video failure reported with xv.
This is because there is a request to do both plane scaling
and colorkey. Because skl hw doesn't support plane scaling
and colorkey at the same time, request is failed which is expected
behavior.
To make xv operate, this patch allows colorkey continue to work
without using scaler. Then behavior would be similar to platforms
without plane scaler support.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
---
drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++++++++++++----------
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7ab75c0..fc5cef0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4505,9 +4505,10 @@ skl_update_scaler_users(
}
/* check colorkey */
- if (intel_plane && intel_plane->ckey.flags != I915_SET_COLORKEY_NONE) {
- DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed",
- intel_plane->base.base.id);
+ if (WARN_ON(intel_plane &&
+ intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
+ DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not allowed with colorkey",
+ intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
return -EINVAL;
}
@@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane *plane,
intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
if (INTEL_INFO(dev)->gen >= 9) {
- min_scale = 1;
- max_scale = skl_max_scale(intel_crtc, crtc_state);
+ /* use scaler when colorkey is not required */
+ if (to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
+ min_scale = 1;
+ max_scale = skl_max_scale(intel_crtc, crtc_state);
+ }
can_position = true;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 16d0e48..9004e47 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
struct drm_rect *dst = &state->dst;
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
- int max_scale, min_scale;
+ int max_scale, min_scale, can_scale;
int pixel_size;
int ret;
@@ -800,18 +800,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
return -EINVAL;
}
+ /* setup can_scale, min_scale, max_scale */
+ if (INTEL_INFO(dev)->gen >= 9) {
+ /* use scaler when colorkey is not required */
+ if (intel_plane->ckey.flags == I915_SET_COLORKEY_NONE) {
+ can_scale = 1;
+ min_scale = 1;
+ max_scale = skl_max_scale(intel_crtc, crtc_state);
+ } else {
+ can_scale = 0;
+ min_scale = DRM_PLANE_HELPER_NO_SCALING;
+ max_scale = DRM_PLANE_HELPER_NO_SCALING;
+ }
+ } else {
+ can_scale = intel_plane->can_scale;
+ max_scale = intel_plane->max_downscale << 16;
+ min_scale = intel_plane->can_scale ? 1 : (1 << 16);
+ }
+
/*
* FIXME the following code does a bunch of fuzzy adjustments to the
* coordinates and sizes. We probably need some way to decide whether
* more strict checking should be done instead.
*/
- max_scale = intel_plane->max_downscale << 16;
- min_scale = intel_plane->can_scale ? 1 : (1 << 16);
-
- if (INTEL_INFO(dev)->gen >= 9) {
- min_scale = 1;
- max_scale = skl_max_scale(intel_crtc, crtc_state);
- }
drm_rect_rotate(src, fb->width << 16, fb->height << 16,
state->base.rotation);
@@ -882,7 +893,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
* Must keep src and dst the
* same if we can't scale.
*/
- if (!intel_plane->can_scale)
+ if (!can_scale)
crtc_w &= ~1;
if (crtc_w == 0)
@@ -894,7 +905,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
unsigned int width_bytes;
- WARN_ON(!intel_plane->can_scale);
+ WARN_ON(!can_scale);
/* FIXME interlacing min height is 6 */
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
2015-05-18 23:18 [PATCH] drm/i915/skl: don't fail colorkey + scaler request Chandra Konduru
@ 2015-05-19 23:47 ` shuang.he
2015-05-21 16:05 ` Konduru, Chandra
2015-05-21 16:36 ` Ville Syrjälä
2 siblings, 0 replies; 6+ messages in thread
From: shuang.he @ 2015-05-19 23:47 UTC (permalink / raw)
To: shuang.he, lei.a.liu, intel-gfx, chandra.konduru
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6431
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 234/234 234/234
ILK 262/262 262/262
SNB -1 282/282 281/282
IVB 300/300 300/300
BYT 254/254 254/254
BDW -1 275/275 274/275
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(7)PASS(1) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
*BDW igt@gem_gtt_hog PASS(2) DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane
assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
2015-05-18 23:18 [PATCH] drm/i915/skl: don't fail colorkey + scaler request Chandra Konduru
2015-05-19 23:47 ` shuang.he
@ 2015-05-21 16:05 ` Konduru, Chandra
2015-05-21 16:36 ` Ville Syrjälä
2 siblings, 0 replies; 6+ messages in thread
From: Konduru, Chandra @ 2015-05-21 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Xu, Bin A, Syrjala, Ville, Vetter, Daniel
Bin confirmed that, with the patch ,the issue didn't exist now.
Need a reviewer so the patch can be lined up for merge.
Ville or anyone in cc list?
-Chandra
> -----Original Message-----
> From: Konduru, Chandra
> Sent: Monday, May 18, 2015 4:19 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Konduru, Chandra; Roper, Matthew D; Syrjala, Ville; Vetter, Daniel; Xu, Bin A
> Subject: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
>
> There is a mplayer video failure reported with xv.
> This is because there is a request to do both plane scaling
> and colorkey. Because skl hw doesn't support plane scaling
> and colorkey at the same time, request is failed which is expected
> behavior.
>
> To make xv operate, this patch allows colorkey continue to work
> without using scaler. Then behavior would be similar to platforms
> without plane scaler support.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
> drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++++++++++++----------
> 2 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 7ab75c0..fc5cef0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4505,9 +4505,10 @@ skl_update_scaler_users(
> }
>
> /* check colorkey */
> - if (intel_plane && intel_plane->ckey.flags !=
> I915_SET_COLORKEY_NONE) {
> - DRM_DEBUG_KMS("PLANE:%d scaling with color key not
> allowed",
> - intel_plane->base.base.id);
> + if (WARN_ON(intel_plane &&
> + intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
> + DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not
> allowed with colorkey",
> + intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
> return -EINVAL;
> }
>
> @@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane
> *plane,
> intel_atomic_get_crtc_state(state->base.state, intel_crtc) :
> NULL;
>
> if (INTEL_INFO(dev)->gen >= 9) {
> - min_scale = 1;
> - max_scale = skl_max_scale(intel_crtc, crtc_state);
> + /* use scaler when colorkey is not required */
> + if (to_intel_plane(plane)->ckey.flags ==
> I915_SET_COLORKEY_NONE) {
> + min_scale = 1;
> + max_scale = skl_max_scale(intel_crtc, crtc_state);
> + }
> can_position = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 16d0e48..9004e47 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> struct drm_rect *dst = &state->dst;
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> - int max_scale, min_scale;
> + int max_scale, min_scale, can_scale;
> int pixel_size;
> int ret;
>
> @@ -800,18 +800,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + /* setup can_scale, min_scale, max_scale */
> + if (INTEL_INFO(dev)->gen >= 9) {
> + /* use scaler when colorkey is not required */
> + if (intel_plane->ckey.flags == I915_SET_COLORKEY_NONE) {
> + can_scale = 1;
> + min_scale = 1;
> + max_scale = skl_max_scale(intel_crtc, crtc_state);
> + } else {
> + can_scale = 0;
> + min_scale = DRM_PLANE_HELPER_NO_SCALING;
> + max_scale = DRM_PLANE_HELPER_NO_SCALING;
> + }
> + } else {
> + can_scale = intel_plane->can_scale;
> + max_scale = intel_plane->max_downscale << 16;
> + min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> + }
> +
> /*
> * FIXME the following code does a bunch of fuzzy adjustments to the
> * coordinates and sizes. We probably need some way to decide
> whether
> * more strict checking should be done instead.
> */
> - max_scale = intel_plane->max_downscale << 16;
> - min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> -
> - if (INTEL_INFO(dev)->gen >= 9) {
> - min_scale = 1;
> - max_scale = skl_max_scale(intel_crtc, crtc_state);
> - }
>
> drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> state->base.rotation);
> @@ -882,7 +893,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> * Must keep src and dst the
> * same if we can't scale.
> */
> - if (!intel_plane->can_scale)
> + if (!can_scale)
> crtc_w &= ~1;
>
> if (crtc_w == 0)
> @@ -894,7 +905,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> unsigned int width_bytes;
>
> - WARN_ON(!intel_plane->can_scale);
> + WARN_ON(!can_scale);
>
> /* FIXME interlacing min height is 6 */
>
> --
> 1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
2015-05-18 23:18 [PATCH] drm/i915/skl: don't fail colorkey + scaler request Chandra Konduru
2015-05-19 23:47 ` shuang.he
2015-05-21 16:05 ` Konduru, Chandra
@ 2015-05-21 16:36 ` Ville Syrjälä
2015-05-22 6:29 ` Daniel Vetter
2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2015-05-21 16:36 UTC (permalink / raw)
To: Chandra Konduru; +Cc: bin.a.xu, daniel.vetter, intel-gfx, ville.syrjala
On Mon, May 18, 2015 at 04:18:44PM -0700, Chandra Konduru wrote:
> There is a mplayer video failure reported with xv.
> This is because there is a request to do both plane scaling
> and colorkey. Because skl hw doesn't support plane scaling
> and colorkey at the same time, request is failed which is expected
> behavior.
>
> To make xv operate, this patch allows colorkey continue to work
> without using scaler. Then behavior would be similar to platforms
> without plane scaler support.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
> drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++++++++++++----------
> 2 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7ab75c0..fc5cef0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4505,9 +4505,10 @@ skl_update_scaler_users(
> }
>
> /* check colorkey */
> - if (intel_plane && intel_plane->ckey.flags != I915_SET_COLORKEY_NONE) {
> - DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed",
> - intel_plane->base.base.id);
> + if (WARN_ON(intel_plane &&
> + intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
> + DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not allowed with colorkey",
> + intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
> return -EINVAL;
> }
>
> @@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane *plane,
> intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
>
> if (INTEL_INFO(dev)->gen >= 9) {
> - min_scale = 1;
> - max_scale = skl_max_scale(intel_crtc, crtc_state);
> + /* use scaler when colorkey is not required */
> + if (to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
> + min_scale = 1;
> + max_scale = skl_max_scale(intel_crtc, crtc_state);
> + }
> can_position = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 16d0e48..9004e47 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> struct drm_rect *dst = &state->dst;
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> - int max_scale, min_scale;
> + int max_scale, min_scale, can_scale;
can_scale ought to be bool. Otherwise this looks good.
So with that bikeshed done this gets:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Ps. Please use my @linux.intel.com address when cc:ing me on patches.
I try to shovel all patch traffic through that and use the @intel.com
address for other stuff.
> int pixel_size;
> int ret;
>
> @@ -800,18 +800,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + /* setup can_scale, min_scale, max_scale */
> + if (INTEL_INFO(dev)->gen >= 9) {
> + /* use scaler when colorkey is not required */
> + if (intel_plane->ckey.flags == I915_SET_COLORKEY_NONE) {
> + can_scale = 1;
> + min_scale = 1;
> + max_scale = skl_max_scale(intel_crtc, crtc_state);
> + } else {
> + can_scale = 0;
> + min_scale = DRM_PLANE_HELPER_NO_SCALING;
> + max_scale = DRM_PLANE_HELPER_NO_SCALING;
> + }
> + } else {
> + can_scale = intel_plane->can_scale;
> + max_scale = intel_plane->max_downscale << 16;
> + min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> + }
> +
> /*
> * FIXME the following code does a bunch of fuzzy adjustments to the
> * coordinates and sizes. We probably need some way to decide whether
> * more strict checking should be done instead.
> */
> - max_scale = intel_plane->max_downscale << 16;
> - min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> -
> - if (INTEL_INFO(dev)->gen >= 9) {
> - min_scale = 1;
> - max_scale = skl_max_scale(intel_crtc, crtc_state);
> - }
>
> drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> state->base.rotation);
> @@ -882,7 +893,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> * Must keep src and dst the
> * same if we can't scale.
> */
> - if (!intel_plane->can_scale)
> + if (!can_scale)
> crtc_w &= ~1;
>
> if (crtc_w == 0)
> @@ -894,7 +905,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> unsigned int width_bytes;
>
> - WARN_ON(!intel_plane->can_scale);
> + WARN_ON(!can_scale);
>
> /* FIXME interlacing min height is 6 */
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
2015-05-21 16:36 ` Ville Syrjälä
@ 2015-05-22 6:29 ` Daniel Vetter
2015-05-22 6:56 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2015-05-22 6:29 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: bin.a.xu, daniel.vetter, intel-gfx, ville.syrjala
On Thu, May 21, 2015 at 07:36:44PM +0300, Ville Syrjälä wrote:
> On Mon, May 18, 2015 at 04:18:44PM -0700, Chandra Konduru wrote:
> > There is a mplayer video failure reported with xv.
> > This is because there is a request to do both plane scaling
> > and colorkey. Because skl hw doesn't support plane scaling
> > and colorkey at the same time, request is failed which is expected
> > behavior.
> >
> > To make xv operate, this patch allows colorkey continue to work
> > without using scaler. Then behavior would be similar to platforms
> > without plane scaler support.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
> > drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++++++++++++----------
> > 2 files changed, 30 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7ab75c0..fc5cef0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4505,9 +4505,10 @@ skl_update_scaler_users(
> > }
> >
> > /* check colorkey */
> > - if (intel_plane && intel_plane->ckey.flags != I915_SET_COLORKEY_NONE) {
> > - DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed",
> > - intel_plane->base.base.id);
> > + if (WARN_ON(intel_plane &&
> > + intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
> > + DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not allowed with colorkey",
> > + intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
> > return -EINVAL;
> > }
> >
> > @@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane *plane,
> > intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
> >
> > if (INTEL_INFO(dev)->gen >= 9) {
> > - min_scale = 1;
> > - max_scale = skl_max_scale(intel_crtc, crtc_state);
> > + /* use scaler when colorkey is not required */
> > + if (to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
> > + min_scale = 1;
> > + max_scale = skl_max_scale(intel_crtc, crtc_state);
> > + }
> > can_position = true;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 16d0e48..9004e47 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > struct drm_rect *dst = &state->dst;
> > const struct drm_rect *clip = &state->clip;
> > int hscale, vscale;
> > - int max_scale, min_scale;
> > + int max_scale, min_scale, can_scale;
>
> can_scale ought to be bool. Otherwise this looks good.
> So with that bikeshed done this gets:
Done. Aside, intel_check_sprite_plane is a giant function, some extraction
of sub-parts and helpers (like computing can_scale and stuff) would be
good. A lot of the big local variable list could e.g. be moved into
intel_plane_state (like the dynamic can_scale) and then we just need to
pass around the state pointer to the various subfunctions.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
>
> Ps. Please use my @linux.intel.com address when cc:ing me on patches.
> I try to shovel all patch traffic through that and use the @intel.com
> address for other stuff.
>
> > int pixel_size;
> > int ret;
> >
> > @@ -800,18 +800,29 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > return -EINVAL;
> > }
> >
> > + /* setup can_scale, min_scale, max_scale */
> > + if (INTEL_INFO(dev)->gen >= 9) {
> > + /* use scaler when colorkey is not required */
> > + if (intel_plane->ckey.flags == I915_SET_COLORKEY_NONE) {
> > + can_scale = 1;
> > + min_scale = 1;
> > + max_scale = skl_max_scale(intel_crtc, crtc_state);
> > + } else {
> > + can_scale = 0;
> > + min_scale = DRM_PLANE_HELPER_NO_SCALING;
> > + max_scale = DRM_PLANE_HELPER_NO_SCALING;
> > + }
> > + } else {
> > + can_scale = intel_plane->can_scale;
> > + max_scale = intel_plane->max_downscale << 16;
> > + min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> > + }
> > +
> > /*
> > * FIXME the following code does a bunch of fuzzy adjustments to the
> > * coordinates and sizes. We probably need some way to decide whether
> > * more strict checking should be done instead.
> > */
> > - max_scale = intel_plane->max_downscale << 16;
> > - min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> > -
> > - if (INTEL_INFO(dev)->gen >= 9) {
> > - min_scale = 1;
> > - max_scale = skl_max_scale(intel_crtc, crtc_state);
> > - }
> >
> > drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> > state->base.rotation);
> > @@ -882,7 +893,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > * Must keep src and dst the
> > * same if we can't scale.
> > */
> > - if (!intel_plane->can_scale)
> > + if (!can_scale)
> > crtc_w &= ~1;
> >
> > if (crtc_w == 0)
> > @@ -894,7 +905,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> > unsigned int width_bytes;
> >
> > - WARN_ON(!intel_plane->can_scale);
> > + WARN_ON(!can_scale);
> >
> > /* FIXME interlacing min height is 6 */
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/skl: don't fail colorkey + scaler request
2015-05-22 6:29 ` Daniel Vetter
@ 2015-05-22 6:56 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2015-05-22 6:56 UTC (permalink / raw)
To: Daniel Vetter; +Cc: bin.a.xu, daniel.vetter, intel-gfx, ville.syrjala
On Fri, May 22, 2015 at 08:29:28AM +0200, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 07:36:44PM +0300, Ville Syrjälä wrote:
> > On Mon, May 18, 2015 at 04:18:44PM -0700, Chandra Konduru wrote:
> > > There is a mplayer video failure reported with xv.
> > > This is because there is a request to do both plane scaling
> > > and colorkey. Because skl hw doesn't support plane scaling
> > > and colorkey at the same time, request is failed which is expected
> > > behavior.
> > >
> > > To make xv operate, this patch allows colorkey continue to work
> > > without using scaler. Then behavior would be similar to platforms
> > > without plane scaler support.
> > >
> > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90449
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 14 +++++++++-----
> > > drivers/gpu/drm/i915/intel_sprite.c | 31 +++++++++++++++++++++----------
> > > 2 files changed, 30 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 7ab75c0..fc5cef0 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4505,9 +4505,10 @@ skl_update_scaler_users(
> > > }
> > >
> > > /* check colorkey */
> > > - if (intel_plane && intel_plane->ckey.flags != I915_SET_COLORKEY_NONE) {
> > > - DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed",
> > > - intel_plane->base.base.id);
> > > + if (WARN_ON(intel_plane &&
> > > + intel_plane->ckey.flags != I915_SET_COLORKEY_NONE)) {
> > > + DRM_DEBUG_KMS("PLANE:%d scaling %ux%u->%ux%u not allowed with colorkey",
> > > + intel_plane->base.base.id, src_w, src_h, dst_w, dst_h);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -13049,8 +13050,11 @@ intel_check_primary_plane(struct drm_plane *plane,
> > > intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
> > >
> > > if (INTEL_INFO(dev)->gen >= 9) {
> > > - min_scale = 1;
> > > - max_scale = skl_max_scale(intel_crtc, crtc_state);
> > > + /* use scaler when colorkey is not required */
> > > + if (to_intel_plane(plane)->ckey.flags == I915_SET_COLORKEY_NONE) {
> > > + min_scale = 1;
> > > + max_scale = skl_max_scale(intel_crtc, crtc_state);
> > > + }
> > > can_position = true;
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 16d0e48..9004e47 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -775,7 +775,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> > > struct drm_rect *dst = &state->dst;
> > > const struct drm_rect *clip = &state->clip;
> > > int hscale, vscale;
> > > - int max_scale, min_scale;
> > > + int max_scale, min_scale, can_scale;
> >
> > can_scale ought to be bool. Otherwise this looks good.
> > So with that bikeshed done this gets:
>
> Done. Aside, intel_check_sprite_plane is a giant function, some extraction
> of sub-parts and helpers (like computing can_scale and stuff) would be
> good.
And some of it should be split up into platform specific checks
since not all platforms have the same restrictions. I think what's
there is generally valid for ILK-IVB but may not be really valid for
others.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-22 6:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18 23:18 [PATCH] drm/i915/skl: don't fail colorkey + scaler request Chandra Konduru
2015-05-19 23:47 ` shuang.he
2015-05-21 16:05 ` Konduru, Chandra
2015-05-21 16:36 ` Ville Syrjälä
2015-05-22 6:29 ` Daniel Vetter
2015-05-22 6:56 ` Ville Syrjälä
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.