| Message ID | 20180529182804.8571-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2018-05-29 at 21:28 +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On SKL+ the dst colorkey must be configured on the lower > plane that contains the colorkey. This is in contrast to > most earlier platforms where the dst colorkey is configured > on the plane above. > > The hardware will peform dst keying only between two immediately > adjacent (in zorder) planes. Plane 1 will be keyed against plane 0, > plane 2 againts plane 1, and so on. There is no way to key arbitrary > planes against plane 0. Thus offering dst color keying on plane 2+ > is pointless. In fact it can be harmful since enabling dst keying on > more than one plane on the same pipe leads to only the top-most of > the planes performing the keying. For any plane lower in zorder the > dst key enable is simply ignored. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 63 > +++++++++++++++++++++++++++++++++++-- > 1 file changed, 60 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index ee23613f9fd4..6164c2ca20c3 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane > *plane, > return 0; > } > > +static bool has_dst_key_in_primary_plane(struct drm_i915_private > *dev_priv) > +{ > + return INTEL_GEN(dev_priv) >= 9; > +} > + > +static void intel_plane_set_ckey(struct intel_plane_state > *plane_state, > + const struct > drm_intel_sprite_colorkey *set) > +{ > + struct intel_plane *plane = to_intel_plane(plane_state- > >base.plane); > + struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > + > + *key = *set; > + > + /* > + * We want src key enabled on the > + * sprite and not on the primary. > + */ > + if (plane->id == PLANE_PRIMARY && > + set->flags & I915_SET_COLORKEY_SOURCE) > + key->flags = 0; > + > + /* > + * On SKL+ we want dst key enabled on > + * the primary and not on the sprite. > + */ > + if (plane->id != PLANE_PRIMARY && > + set->flags & I915_SET_COLORKEY_DESTINATION) > + key->flags = 0; > +} > + > int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void > *data, > struct drm_file *file_priv) > { > @@ -1100,6 +1130,16 @@ int intel_sprite_set_colorkey_ioctl(struct > drm_device *dev, void *data, > if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY) > return -ENOENT; > > + /* > + * SKL+ only plane 1 can do destination keying against plane > 0. > + * Also multiple planes can't do destination keying on the > same > + * pipe simultaneously. > + */ > + if (INTEL_GEN(dev_priv) >= 9 && > + to_intel_plane(plane)->id >= PLANE_SPRITE1 && > + set->flags & I915_SET_COLORKEY_DESTINATION) > + return -EINVAL; > + > drm_modeset_acquire_init(&ctx, 0); > > state = drm_atomic_state_alloc(plane->dev); > @@ -1112,11 +1152,28 @@ int intel_sprite_set_colorkey_ioctl(struct > drm_device *dev, void *data, > while (1) { > plane_state = drm_atomic_get_plane_state(state, > plane); > ret = PTR_ERR_OR_ZERO(plane_state); > - if (!ret) { > - to_intel_plane_state(plane_state)->ckey = > *set; > - ret = drm_atomic_commit(state); > + if (!ret) > + intel_plane_set_ckey(to_intel_plane_state(pl > ane_state), set); > + > + /* > + * On some platforms we have to configure > + * the dst colorkey on the primary plane. > + */ > + if (!ret && has_dst_key_in_primary_plane(dev_priv)) > { > + struct intel_crtc *crtc = > + intel_get_crtc_for_pipe(dev_priv, > + to_intel_pla > ne(plane)->pipe); > + > + plane_state = > drm_atomic_get_plane_state(state, > + crt > c->base.primary); > + ret = PTR_ERR_OR_ZERO(plane_state); > + if (!ret) > + intel_plane_set_ckey(to_intel_plane_ > state(plane_state), set); > } > > + if (!ret) > + ret = drm_atomic_commit(state); > + > if (ret != -EDEADLK) > break; > -- Best Regards, Lisovskiy Stanislav
On Tue, Jun 05, 2018 at 08:10:36AM +0000, Lisovskiy, Stanislav wrote: > On Tue, 2018-05-29 at 21:28 +0300, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > On SKL+ the dst colorkey must be configured on the lower > > plane that contains the colorkey. This is in contrast to > > most earlier platforms where the dst colorkey is configured > > on the plane above. > > > > The hardware will peform dst keying only between two immediately > > adjacent (in zorder) planes. Plane 1 will be keyed against plane 0, > > plane 2 againts plane 1, and so on. There is no way to key arbitrary > > planes against plane 0. Thus offering dst color keying on plane 2+ > > is pointless. In fact it can be harmful since enabling dst keying on > > more than one plane on the same pipe leads to only the top-most of > > the planes performing the keying. For any plane lower in zorder the > > dst key enable is simply ignored. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 63 > > +++++++++++++++++++++++++++++++++++-- > > 1 file changed, 60 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > b/drivers/gpu/drm/i915/intel_sprite.c > > index ee23613f9fd4..6164c2ca20c3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane > > *plane, > > return 0; > > } > > > > +static bool has_dst_key_in_primary_plane(struct drm_i915_private > > *dev_priv) > > +{ > > + return INTEL_GEN(dev_priv) >= 9; > > +} > > + > > +static void intel_plane_set_ckey(struct intel_plane_state > > *plane_state, > > + const struct > > drm_intel_sprite_colorkey *set) > > +{ > > + struct intel_plane *plane = to_intel_plane(plane_state- > > >base.plane); > > + struct drm_intel_sprite_colorkey *key = &plane_state->ckey; > > + > > + *key = *set; > > + > > + /* > > + * We want src key enabled on the > > + * sprite and not on the primary. > > + */ > > + if (plane->id == PLANE_PRIMARY && > > + set->flags & I915_SET_COLORKEY_SOURCE) > > + key->flags = 0; > > + > > + /* > > + * On SKL+ we want dst key enabled on > > + * the primary and not on the sprite. > > + */ > > + if (plane->id != PLANE_PRIMARY && The 'INTEL_GEN(dev_priv) >= 9' check that was supposed to be here ended up in the wrong patch. I moved it over while applying this to avoid breaking the pre-SKL sprites. Only caught it when glancing at the resulting code before pushing. Might be time to write some real colorkeying igts... Patch pushed to dinq. Thanks for the review. > > + set->flags & I915_SET_COLORKEY_DESTINATION) > > + key->flags = 0; > > +} > > +
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index ee23613f9fd4..6164c2ca20c3 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1071,6 +1071,36 @@ intel_check_sprite_plane(struct intel_plane *plane, return 0; } +static bool has_dst_key_in_primary_plane(struct drm_i915_private *dev_priv) +{ + return INTEL_GEN(dev_priv) >= 9; +} + +static void intel_plane_set_ckey(struct intel_plane_state *plane_state, + const struct drm_intel_sprite_colorkey *set) +{ + struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + struct drm_intel_sprite_colorkey *key = &plane_state->ckey; + + *key = *set; + + /* + * We want src key enabled on the + * sprite and not on the primary. + */ + if (plane->id == PLANE_PRIMARY && + set->flags & I915_SET_COLORKEY_SOURCE) + key->flags = 0; + + /* + * On SKL+ we want dst key enabled on + * the primary and not on the sprite. + */ + if (plane->id != PLANE_PRIMARY && + set->flags & I915_SET_COLORKEY_DESTINATION) + key->flags = 0; +} + int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1100,6 +1130,16 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data, if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY) return -ENOENT; + /* + * SKL+ only plane 1 can do destination keying against plane 0. + * Also multiple planes can't do destination keying on the same + * pipe simultaneously. + */ + if (INTEL_GEN(dev_priv) >= 9 && + to_intel_plane(plane)->id >= PLANE_SPRITE1 && + set->flags & I915_SET_COLORKEY_DESTINATION) + return -EINVAL; + drm_modeset_acquire_init(&ctx, 0); state = drm_atomic_state_alloc(plane->dev); @@ -1112,11 +1152,28 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data, while (1) { plane_state = drm_atomic_get_plane_state(state, plane); ret = PTR_ERR_OR_ZERO(plane_state); - if (!ret) { - to_intel_plane_state(plane_state)->ckey = *set; - ret = drm_atomic_commit(state); + if (!ret) + intel_plane_set_ckey(to_intel_plane_state(plane_state), set); + + /* + * On some platforms we have to configure + * the dst colorkey on the primary plane. + */ + if (!ret && has_dst_key_in_primary_plane(dev_priv)) { + struct intel_crtc *crtc = + intel_get_crtc_for_pipe(dev_priv, + to_intel_plane(plane)->pipe); + + plane_state = drm_atomic_get_plane_state(state, + crtc->base.primary); + ret = PTR_ERR_OR_ZERO(plane_state); + if (!ret) + intel_plane_set_ckey(to_intel_plane_state(plane_state), set); } + if (!ret) + ret = drm_atomic_commit(state); + if (ret != -EDEADLK) break;