On Thu, Apr 09, 2015 at 02:46:19PM +0200, Daniel Vetter wrote:
> On Wed, Apr 08, 2015 at 06:56:53PM -0700, Matt Roper wrote:
> > Switch from our plane update/disable entrypoints to use the full atomic
> > helpers (which generate a top-level atomic transaction) rather than the
> > transitional helpers (which only create/manipulate orphaned plane states
> > independent of a top-level transaction). Various upcoming work (SKL
> > scalers, atomic watermarks, etc.) requires a full atomic transaction to
> > behave properly/cleanly.
> >
> > Last time we tried this, we had to back out the change because we still
> > call the drm_plane vfuncs directly from within our legacy modesetting
> > code. This potentially results in nested atomic transactions, locking
> > collisions, and other failures. To avoid that problem again, we
> > sidestep the issue by calling the transitional helpers directly (rather
> > than through a vfunc) when we're nested inside of other legacy
> > modesetting code. However this does allow legacy SetPlane() ioctl's to
> > process an entire drm_atomic_state transaction, which is important for
> > upcoming patches.
> >
> > Cc: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
> Maarten is working to fix this properly (by wiring up plane states to the
> transitional drm_atomic_state scaffolding from Ander), but seems like a
> good interim idea to get back some solid testing for our atomic code.
>
> Can I apply this without patches 1&2 right away or is there a tricky
> depency?
> -Daniel
I think this one can be applied independently of 1&2. I think this one
might be a prereq for #4 to fully work as advertised though...without
using the full atomic helpers, we simply don't have a 'start of
transaction' point at which we can clear the existing atomic flags when
using legacy plane ioctls.
Matt
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++------------
> > drivers/gpu/drm/i915/intel_sprite.c | 10 +++++-----
> > 2 files changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1cf91ad..3a74923 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5576,7 +5576,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
> > dev_priv->display.crtc_disable(crtc);
> > dev_priv->display.off(crtc);
> >
> > - crtc->primary->funcs->disable_plane(crtc->primary);
> > + drm_plane_helper_disable(crtc->primary);
> >
> > /* Update computed state. */
> > list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> > @@ -11731,11 +11731,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> > int vdisplay, hdisplay;
> >
> > drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
> > - ret = primary->funcs->update_plane(primary, &intel_crtc->base,
> > - fb, 0, 0,
> > - hdisplay, vdisplay,
> > - x << 16, y << 16,
> > - hdisplay << 16, vdisplay << 16);
> > + ret = drm_plane_helper_update(primary, &intel_crtc->base,
> > + fb, 0, 0,
> > + hdisplay, vdisplay,
> > + x << 16, y << 16,
> > + hdisplay << 16, vdisplay << 16);
> > }
> >
> > /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> > @@ -12286,10 +12286,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > int vdisplay, hdisplay;
> >
> > drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
> > - ret = primary->funcs->update_plane(primary, set->crtc, set->fb,
> > - 0, 0, hdisplay, vdisplay,
> > - set->x << 16, set->y << 16,
> > - hdisplay << 16, vdisplay << 16);
> > + ret = drm_plane_helper_update(primary, set->crtc, set->fb,
> > + 0, 0, hdisplay, vdisplay,
> > + set->x << 16, set->y << 16,
> > + hdisplay << 16, vdisplay << 16);
> >
> > /*
> > * We need to make sure the primary plane is re-enabled if it
> > @@ -12830,8 +12830,8 @@ void intel_plane_destroy(struct drm_plane *plane)
> > }
> >
> > const struct drm_plane_funcs intel_plane_funcs = {
> > - .update_plane = drm_plane_helper_update,
> > - .disable_plane = drm_plane_helper_disable,
> > + .update_plane = drm_atomic_helper_update_plane,
> > + .disable_plane = drm_atomic_helper_disable_plane,
> > .destroy = intel_plane_destroy,
> > .set_property = drm_atomic_helper_plane_set_property,
> > .atomic_get_property = intel_plane_atomic_get_property,
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 492abcd..f4094d2 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1149,11 +1149,11 @@ int intel_plane_restore(struct drm_plane *plane)
> > if (!plane->crtc || !plane->state->fb)
> > return 0;
> >
> > - return plane->funcs->update_plane(plane, plane->crtc, plane->state->fb,
> > - plane->state->crtc_x, plane->state->crtc_y,
> > - plane->state->crtc_w, plane->state->crtc_h,
> > - plane->state->src_x, plane->state->src_y,
> > - plane->state->src_w, plane->state->src_h);
> > + return drm_plane_helper_update(plane, plane->crtc, plane->state->fb,
> > + plane->state->crtc_x, plane->state->crtc_y,
> > + plane->state->crtc_w, plane->state->crtc_h,
> > + plane->state->src_x, plane->state->src_y,
> > + plane->state->src_w, plane->state->src_h);
> > }
> >
> > static uint32_t ilk_plane_formats[] = {
> > --
> > 1.8.5.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
