| Message ID | 1415904206-22548-3-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 13 Nov 2014 10:43:21 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > We'll want to call this from the type-agnostic atomic plane helper > hooks. Since it's not sprite-specific anymore, more it to > intel_display.c as well. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > 3 files changed, 24 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a9f90b8..c6598e9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > spin_unlock_irq(&dev->event_lock); > } > } > + > +void intel_plane_disable(struct drm_plane *plane) > +{ > + if (!plane->crtc || !plane->fb) > + return; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + intel_primary_plane_disable(plane); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + intel_cursor_plane_disable(plane); > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + intel_disable_plane(plane); > + break; > + default: > + WARN(1, "Unknown plane type"); > + } > +} > + > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index bd5ef4e..df1420b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > void intel_finish_page_flip(struct drm_device *dev, int pipe); > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > void intel_check_page_flip(struct drm_device *dev, int pipe); > +void intel_plane_disable(struct drm_plane *plane); > > /* shared dpll functions */ > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > struct drm_property *prop, > uint64_t val); > int intel_plane_restore(struct drm_plane *plane); > -void intel_plane_disable(struct drm_plane *plane); > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > struct drm_file *file_priv); > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > bool intel_pipe_update_start(struct intel_crtc *crtc, > uint32_t *start_vbl_count); > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > +int intel_disable_plane(struct drm_plane *plane); Would it make sense to rename this to intel_sprite_plane_disable() as part of this? It would be more consistent with the cursor and primary plane naming conventions and likely avoid some confusion with the intel_plane_disable() function. > > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index fc96d13..115acd3 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > return 0; > } > > -static int > +int > intel_disable_plane(struct drm_plane *plane) > { > struct drm_device *dev = plane->dev; > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > intel_plane->src_w, intel_plane->src_h); > } > > -void intel_plane_disable(struct drm_plane *plane) > -{ > - if (!plane->crtc || !plane->fb) > - return; > - > - intel_disable_plane(plane); > -} > - > static const struct drm_plane_funcs intel_plane_funcs = { > .update_plane = intel_update_plane, > .disable_plane = intel_disable_plane,
On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > On Thu, 13 Nov 2014 10:43:21 -0800 > Matt Roper <matthew.d.roper@intel.com> wrote: > > > We'll want to call this from the type-agnostic atomic plane helper > > hooks. Since it's not sprite-specific anymore, more it to > > intel_display.c as well. > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a9f90b8..c6598e9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > spin_unlock_irq(&dev->event_lock); > > } > > } > > + > > +void intel_plane_disable(struct drm_plane *plane) > > +{ > > + if (!plane->crtc || !plane->fb) > > + return; > > + > > + switch (plane->type) { > > + case DRM_PLANE_TYPE_PRIMARY: > > + intel_primary_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_CURSOR: > > + intel_cursor_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_OVERLAY: > > + intel_disable_plane(plane); > > + break; > > + default: > > + WARN(1, "Unknown plane type"); > > + } > > +} > > + > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index bd5ef4e..df1420b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > void intel_finish_page_flip(struct drm_device *dev, int pipe); > > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > > void intel_check_page_flip(struct drm_device *dev, int pipe); > > +void intel_plane_disable(struct drm_plane *plane); > > > > /* shared dpll functions */ > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > struct drm_property *prop, > > uint64_t val); > > int intel_plane_restore(struct drm_plane *plane); > > -void intel_plane_disable(struct drm_plane *plane); > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > uint32_t *start_vbl_count); > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > +int intel_disable_plane(struct drm_plane *plane); > > Would it make sense to rename this to intel_sprite_plane_disable() as > part of this? It would be more consistent with the cursor and primary > plane naming conventions and likely avoid some confusion with the > intel_plane_disable() function. > Yeah, that happens in the next patch of the series (along with some other naming cleanup). I tripped over the confusing names too many times while working on this... Matt > > > > /* intel_tv.c */ > > void intel_tv_init(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index fc96d13..115acd3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > return 0; > > } > > > > -static int > > +int > > intel_disable_plane(struct drm_plane *plane) > > { > > struct drm_device *dev = plane->dev; > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > intel_plane->src_w, intel_plane->src_h); > > } > > > > -void intel_plane_disable(struct drm_plane *plane) > > -{ > > - if (!plane->crtc || !plane->fb) > > - return; > > - > > - intel_disable_plane(plane); > > -} > > - > > static const struct drm_plane_funcs intel_plane_funcs = { > > .update_plane = intel_update_plane, > > .disable_plane = intel_disable_plane, >
On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > On Thu, 13 Nov 2014 10:43:21 -0800 > Matt Roper <matthew.d.roper@intel.com> wrote: > > > We'll want to call this from the type-agnostic atomic plane helper > > hooks. Since it's not sprite-specific anymore, more it to > > intel_display.c as well. > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a9f90b8..c6598e9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > spin_unlock_irq(&dev->event_lock); > > } > > } > > + > > +void intel_plane_disable(struct drm_plane *plane) > > +{ > > + if (!plane->crtc || !plane->fb) > > + return; > > + > > + switch (plane->type) { > > + case DRM_PLANE_TYPE_PRIMARY: > > + intel_primary_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_CURSOR: > > + intel_cursor_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_OVERLAY: > > + intel_disable_plane(plane); > > + break; > > + default: > > + WARN(1, "Unknown plane type"); > > + } > > +} > > + > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index bd5ef4e..df1420b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > void intel_finish_page_flip(struct drm_device *dev, int pipe); > > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > > void intel_check_page_flip(struct drm_device *dev, int pipe); > > +void intel_plane_disable(struct drm_plane *plane); > > > > /* shared dpll functions */ > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > struct drm_property *prop, > > uint64_t val); > > int intel_plane_restore(struct drm_plane *plane); > > -void intel_plane_disable(struct drm_plane *plane); > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > uint32_t *start_vbl_count); > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > +int intel_disable_plane(struct drm_plane *plane); > > Would it make sense to rename this to intel_sprite_plane_disable() as > part of this? It would be more consistent with the cursor and primary > plane naming conventions and likely avoid some confusion with the > intel_plane_disable() function. Hmm. Why do we even still have some kind of disable hook in the atomic age? I would expect to just have .commit() or somesuch thing. But is there's still some need for a disable hook, shouldn't we just call the .disable_plane() hook of drm_plane? I guess I should really go and read some this new helper stuff... > > > > /* intel_tv.c */ > > void intel_tv_init(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index fc96d13..115acd3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > return 0; > > } > > > > -static int > > +int > > intel_disable_plane(struct drm_plane *plane) > > { > > struct drm_device *dev = plane->dev; > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > intel_plane->src_w, intel_plane->src_h); > > } > > > > -void intel_plane_disable(struct drm_plane *plane) > > -{ > > - if (!plane->crtc || !plane->fb) > > - return; > > - > > - intel_disable_plane(plane); > > -} > > - > > static const struct drm_plane_funcs intel_plane_funcs = { > > .update_plane = intel_update_plane, > > .disable_plane = intel_disable_plane, > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, 13 Nov 2014 11:12:01 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > > On Thu, 13 Nov 2014 10:43:21 -0800 > > Matt Roper <matthew.d.roper@intel.com> wrote: > > > > > We'll want to call this from the type-agnostic atomic plane helper > > > hooks. Since it's not sprite-specific anymore, more it to > > > intel_display.c as well. > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index a9f90b8..c6598e9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > > spin_unlock_irq(&dev->event_lock); > > > } > > > } > > > + > > > +void intel_plane_disable(struct drm_plane *plane) > > > +{ > > > + if (!plane->crtc || !plane->fb) > > > + return; > > > + > > > + switch (plane->type) { > > > + case DRM_PLANE_TYPE_PRIMARY: > > > + intel_primary_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_CURSOR: > > > + intel_cursor_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_OVERLAY: > > > + intel_disable_plane(plane); > > > + break; > > > + default: > > > + WARN(1, "Unknown plane type"); > > > + } > > > +} > > > + > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index bd5ef4e..df1420b 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > > void intel_finish_page_flip(struct drm_device *dev, int pipe); > > > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > > > void intel_check_page_flip(struct drm_device *dev, int pipe); > > > +void intel_plane_disable(struct drm_plane *plane); > > > > > > /* shared dpll functions */ > > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > > struct drm_property *prop, > > > uint64_t val); > > > int intel_plane_restore(struct drm_plane *plane); > > > -void intel_plane_disable(struct drm_plane *plane); > > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > > struct drm_file *file_priv); > > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > > uint32_t *start_vbl_count); > > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > > +int intel_disable_plane(struct drm_plane *plane); > > > > Would it make sense to rename this to intel_sprite_plane_disable() as > > part of this? It would be more consistent with the cursor and primary > > plane naming conventions and likely avoid some confusion with the > > intel_plane_disable() function. > > > > Yeah, that happens in the next patch of the series (along with some > other naming cleanup). I tripped over the confusing names too many > times while working on this... > > > Matt I tripped over it on my first pass reading through this which is why I think it might be better as part of this patch. But being part of the next patch is fine too. Bob > > > > > > > > /* intel_tv.c */ > > > void intel_tv_init(struct drm_device *dev); > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index fc96d13..115acd3 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > > return 0; > > > } > > > > > > -static int > > > +int > > > intel_disable_plane(struct drm_plane *plane) > > > { > > > struct drm_device *dev = plane->dev; > > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > > intel_plane->src_w, intel_plane->src_h); > > > } > > > > > > -void intel_plane_disable(struct drm_plane *plane) > > > -{ > > > - if (!plane->crtc || !plane->fb) > > > - return; > > > - > > > - intel_disable_plane(plane); > > > -} > > > - > > > static const struct drm_plane_funcs intel_plane_funcs = { > > > .update_plane = intel_update_plane, > > > .disable_plane = intel_disable_plane, > > >
On Thu, Nov 13, 2014 at 09:23:02PM +0200, Ville Syrjälä wrote: > On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > > On Thu, 13 Nov 2014 10:43:21 -0800 > > Matt Roper <matthew.d.roper@intel.com> wrote: > > > > > We'll want to call this from the type-agnostic atomic plane helper > > > hooks. Since it's not sprite-specific anymore, more it to > > > intel_display.c as well. > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index a9f90b8..c6598e9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > > spin_unlock_irq(&dev->event_lock); > > > } > > > } > > > + > > > +void intel_plane_disable(struct drm_plane *plane) > > > +{ > > > + if (!plane->crtc || !plane->fb) > > > + return; > > > + > > > + switch (plane->type) { > > > + case DRM_PLANE_TYPE_PRIMARY: > > > + intel_primary_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_CURSOR: > > > + intel_cursor_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_OVERLAY: > > > + intel_disable_plane(plane); > > > + break; > > > + default: > > > + WARN(1, "Unknown plane type"); > > > + } > > > +} > > > + > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index bd5ef4e..df1420b 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > > void intel_finish_page_flip(struct drm_device *dev, int pipe); > > > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > > > void intel_check_page_flip(struct drm_device *dev, int pipe); > > > +void intel_plane_disable(struct drm_plane *plane); > > > > > > /* shared dpll functions */ > > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > > struct drm_property *prop, > > > uint64_t val); > > > int intel_plane_restore(struct drm_plane *plane); > > > -void intel_plane_disable(struct drm_plane *plane); > > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > > struct drm_file *file_priv); > > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > > uint32_t *start_vbl_count); > > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > > +int intel_disable_plane(struct drm_plane *plane); > > > > Would it make sense to rename this to intel_sprite_plane_disable() as > > part of this? It would be more consistent with the cursor and primary > > plane naming conventions and likely avoid some confusion with the > > intel_plane_disable() function. > > Hmm. Why do we even still have some kind of disable hook in the atomic > age? I would expect to just have .commit() or somesuch thing. > > But is there's still some need for a disable hook, shouldn't we just call > the .disable_plane() hook of drm_plane? I guess I should really go and > read some this new helper stuff... You're right that it does eventually come down to drm_plane_helper_commit() --> intel_plane_atomic_update() which should be able to program the plane to either a new state or off. To reuse as much of our existing code as possible and keep my changes minimal, I'm just implementing intel_plane_atomic_update() as: if (!plane->state->fb) intel_plane_disable(plane); else intel_plane->commit_plane(plane, intel_state); since we already had both of those functions implemented and working. intel_plane->commit_plane() doesn't expect to be called in the disabled case today (assumes crtc and/or fb are non-NULL), but it definitely seems reasonable/easy to tweak that and roll the disable logic directly into the lowest-level commit_plane in the future. I just wanted to keep things as simple as possible with minimal code changes for the initial patchset. Matt > > > > > > > /* intel_tv.c */ > > > void intel_tv_init(struct drm_device *dev); > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index fc96d13..115acd3 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > > return 0; > > > } > > > > > > -static int > > > +int > > > intel_disable_plane(struct drm_plane *plane) > > > { > > > struct drm_device *dev = plane->dev; > > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > > intel_plane->src_w, intel_plane->src_h); > > > } > > > > > > -void intel_plane_disable(struct drm_plane *plane) > > > -{ > > > - if (!plane->crtc || !plane->fb) > > > - return; > > > - > > > - intel_disable_plane(plane); > > > -} > > > - > > > static const struct drm_plane_funcs intel_plane_funcs = { > > > .update_plane = intel_update_plane, > > > .disable_plane = intel_disable_plane, > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a9f90b8..c6598e9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) spin_unlock_irq(&dev->event_lock); } } + +void intel_plane_disable(struct drm_plane *plane) +{ + if (!plane->crtc || !plane->fb) + return; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + intel_primary_plane_disable(plane); + break; + case DRM_PLANE_TYPE_CURSOR: + intel_cursor_plane_disable(plane); + break; + case DRM_PLANE_TYPE_OVERLAY: + intel_disable_plane(plane); + break; + default: + WARN(1, "Unknown plane type"); + } +} + diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bd5ef4e..df1420b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); void intel_finish_page_flip(struct drm_device *dev, int pipe); void intel_finish_page_flip_plane(struct drm_device *dev, int plane); void intel_check_page_flip(struct drm_device *dev, int pipe); +void intel_plane_disable(struct drm_plane *plane); /* shared dpll functions */ struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, struct drm_property *prop, uint64_t val); int intel_plane_restore(struct drm_plane *plane); -void intel_plane_disable(struct drm_plane *plane); int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv); int intel_sprite_get_colorkey(struct drm_device *dev, void *data, @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, bool intel_pipe_update_start(struct intel_crtc *crtc, uint32_t *start_vbl_count); void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); +int intel_disable_plane(struct drm_plane *plane); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fc96d13..115acd3 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, return 0; } -static int +int intel_disable_plane(struct drm_plane *plane) { struct drm_device *dev = plane->dev; @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) intel_plane->src_w, intel_plane->src_h); } -void intel_plane_disable(struct drm_plane *plane) -{ - if (!plane->crtc || !plane->fb) - return; - - intel_disable_plane(plane); -} - static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, .disable_plane = intel_disable_plane,
We'll want to call this from the type-agnostic atomic plane helper hooks. Since it's not sprite-specific anymore, more it to intel_display.c as well. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- 3 files changed, 24 insertions(+), 10 deletions(-)