[Intel-gfx] [PATCH] drm/i915/skl: don't fail colorkey + scaler request
Daniel Vetter
daniel at ffwll.ch
Thu May 21 23:29:28 PDT 2015
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 at 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 at 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list