| Message ID | 1426398946-5900-13-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Mar 14, 2015 at 10:55:37PM -0700, Chandra Konduru wrote: > Plane scaling and colorkey are mutually exclusive. Ensure scaling > isn't active at the time of enabling colorkey. > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index c010528..0194390 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane *drm_plane, > const int plane = intel_plane->plane; > u32 plane_ctl; > > + /* plane scaling and colorkey are mutually exclusive */ > + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { > + DRM_ERROR("colorkey not allowed with scaler\n"); > + return -EINVAL; > + } Yeah this is a bit trouble because of the interactions between colorkey state in hw and the atomic scaler state in the atomic state structures. Since colorkey isn't support with atomic atm anyway and we want to move towards atomic: Do we really have anyone who cares about legacy sprite ioctls on skl? If we'd just add a if (gen >= 9) return -ENODEV; to all the sprite ioctls then we won't have any problems. And we also don't need to add temporary code like the above. -Daniel > + > I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value); > I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value); > I915_WRITE(PLANE_KEYMSK(pipe, plane), key->channel_mask); > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Mar 17, 2015 at 03:16:01PM +0100, Daniel Vetter wrote: > On Sat, Mar 14, 2015 at 10:55:37PM -0700, Chandra Konduru wrote: > > Plane scaling and colorkey are mutually exclusive. Ensure scaling > > isn't active at the time of enabling colorkey. > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index c010528..0194390 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane *drm_plane, > > const int plane = intel_plane->plane; > > u32 plane_ctl; > > > > + /* plane scaling and colorkey are mutually exclusive */ > > + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { > > + DRM_ERROR("colorkey not allowed with scaler\n"); > > + return -EINVAL; > > + } > > Yeah this is a bit trouble because of the interactions between colorkey > state in hw and the atomic scaler state in the atomic state structures. > > Since colorkey isn't support with atomic atm anyway and we want to move > towards atomic: Do we really have anyone who cares about legacy sprite > ioctls on skl? If we'd just add a Yes. Current userspace uses sprites and colorkey. -Chris
On Tue, Mar 17, 2015 at 02:16:33PM +0000, Chris Wilson wrote: > On Tue, Mar 17, 2015 at 03:16:01PM +0100, Daniel Vetter wrote: > > On Sat, Mar 14, 2015 at 10:55:37PM -0700, Chandra Konduru wrote: > > > Plane scaling and colorkey are mutually exclusive. Ensure scaling > > > isn't active at the time of enabling colorkey. > > > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index c010528..0194390 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane *drm_plane, > > > const int plane = intel_plane->plane; > > > u32 plane_ctl; > > > > > > + /* plane scaling and colorkey are mutually exclusive */ > > > + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { > > > + DRM_ERROR("colorkey not allowed with scaler\n"); > > > + return -EINVAL; > > > + } > > > > Yeah this is a bit trouble because of the interactions between colorkey > > state in hw and the atomic scaler state in the atomic state structures. > > > > Since colorkey isn't support with atomic atm anyway and we want to move > > towards atomic: Do we really have anyone who cares about legacy sprite > > ioctls on skl? If we'd just add a > > Yes. Current userspace uses sprites and colorkey. Yes I know we have the code around and it'll keep working. This was more a question of priority and whether we really should keep it working or whether cros/android don't really care. Since sooner or later we need to convert it to atomic/properties if we want to keep it working. Right now (since skl is still prelimary support) we can still do that I hope ... -Daniel
On Tue, Mar 17, 2015 at 03:38:28PM +0100, Daniel Vetter wrote: > On Tue, Mar 17, 2015 at 02:16:33PM +0000, Chris Wilson wrote: > > On Tue, Mar 17, 2015 at 03:16:01PM +0100, Daniel Vetter wrote: > > > On Sat, Mar 14, 2015 at 10:55:37PM -0700, Chandra Konduru wrote: > > > > Plane scaling and colorkey are mutually exclusive. Ensure scaling > > > > isn't active at the time of enabling colorkey. > > > > > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > > index c010528..0194390 100644 > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > > @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane *drm_plane, > > > > const int plane = intel_plane->plane; > > > > u32 plane_ctl; > > > > > > > > + /* plane scaling and colorkey are mutually exclusive */ > > > > + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { > > > > + DRM_ERROR("colorkey not allowed with scaler\n"); > > > > + return -EINVAL; > > > > + } > > > > > > Yeah this is a bit trouble because of the interactions between colorkey > > > state in hw and the atomic scaler state in the atomic state structures. > > > > > > Since colorkey isn't support with atomic atm anyway and we want to move > > > towards atomic: Do we really have anyone who cares about legacy sprite > > > ioctls on skl? If we'd just add a > > > > Yes. Current userspace uses sprites and colorkey. > > Yes I know we have the code around and it'll keep working. This was more a > question of priority and whether we really should keep it working or > whether cros/android don't really care. Since sooner or later we need to > convert it to atomic/properties if we want to keep it working. Right now > (since skl is still prelimary support) we can still do that I hope ... Code was written a couple of years ago with the intent that it would work for the next 5 years across multiple hardware releases. -Chris
> -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Tuesday, March 17, 2015 8:13 AM > To: Daniel Vetter > Cc: Konduru, Chandra; Conselvan De Oliveira, Ander; intel- > gfx@lists.freedesktop.org; Vetter, Daniel > Subject: Re: [Intel-gfx] [PATCH 12/21] drm/i915: Ensure colorkey and scaling > aren't enabled at same time > > On Tue, Mar 17, 2015 at 03:38:28PM +0100, Daniel Vetter wrote: > > On Tue, Mar 17, 2015 at 02:16:33PM +0000, Chris Wilson wrote: > > > On Tue, Mar 17, 2015 at 03:16:01PM +0100, Daniel Vetter wrote: > > > > On Sat, Mar 14, 2015 at 10:55:37PM -0700, Chandra Konduru wrote: > > > > > Plane scaling and colorkey are mutually exclusive. Ensure > > > > > scaling isn't active at the time of enabling colorkey. > > > > > > > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > > > > b/drivers/gpu/drm/i915/intel_sprite.c > > > > > index c010528..0194390 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > > > @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane > *drm_plane, > > > > > const int plane = intel_plane->plane; > > > > > u32 plane_ctl; > > > > > > > > > > + /* plane scaling and colorkey are mutually exclusive */ > > > > > + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { > > > > > + DRM_ERROR("colorkey not allowed with scaler\n"); > > > > > + return -EINVAL; > > > > > + } > > > > > > > > Yeah this is a bit trouble because of the interactions between > > > > colorkey state in hw and the atomic scaler state in the atomic state > structures. > > > > > > > > Since colorkey isn't support with atomic atm anyway and we want to > > > > move towards atomic: Do we really have anyone who cares about > > > > legacy sprite ioctls on skl? If we'd just add a > > > > > > Yes. Current userspace uses sprites and colorkey. > > > > Yes I know we have the code around and it'll keep working. This was > > more a question of priority and whether we really should keep it > > working or whether cros/android don't really care. Since sooner or > > later we need to convert it to atomic/properties if we want to keep it > > working. Right now (since skl is still prelimary support) we can still do that I > hope ... > > Code was written a couple of years ago with the intent that it would work for > the next 5 years across multiple hardware releases. > -Chris Reading color key from hw doesn't harm until sprite colorkey becomes atomic. So keeping this patch as there are users using colorkey. > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index c010528..0194390 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -319,6 +319,12 @@ skl_update_colorkey(struct drm_plane *drm_plane, const int plane = intel_plane->plane; u32 plane_ctl; + /* plane scaling and colorkey are mutually exclusive */ + if (to_intel_plane_state(drm_plane->state)->scaler_id >= 0) { + DRM_ERROR("colorkey not allowed with scaler\n"); + return -EINVAL; + } + I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value); I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value); I915_WRITE(PLANE_KEYMSK(pipe, plane), key->channel_mask);
Plane scaling and colorkey are mutually exclusive. Ensure scaling isn't active at the time of enabling colorkey. Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 6 ++++++ 1 file changed, 6 insertions(+)