| Message ID | 1393539283-5901-5-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 27, 2014 at 02:14:43PM -0800, Matt Roper wrote: > Create a primary plane at CRTC init and hook up handlers for the various > operations that may be performed on it. The DRM core will only > advertise the primary planes to clients that set the appropriate > capability bit. > > Since we're limited to the legacy plane operations at the moment > (SetPlane and such) this isn't terribly interesting yet; the plane > update handler will perform an MMIO flip of the display plane and the > disable handler will disable the CRTC. Once we migrate more of the > plane and CRTC info over to properties in preparation for atomic/nuclear > operations, primary planes will be more useful. > > Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9757010..d9a5cd5 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8260,6 +8260,10 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) > > intel_crtc_cursor_set(crtc, NULL, 0, 0, 0); > > + drm_plane_cleanup(crtc->primary_plane); > + kfree(crtc->primary_plane); > + crtc->primary_plane = NULL; > + > drm_crtc_cleanup(crtc); > > kfree(intel_crtc); > @@ -10272,17 +10276,105 @@ static void intel_shared_dpll_init(struct drm_device *dev) > BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS); > } > > +static int > +intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > + struct drm_framebuffer *fb, int crtc_x, int crtc_y, > + unsigned int crtc_w, unsigned int crtc_h, > + uint32_t src_x, uint32_t src_y, > + uint32_t src_w, uint32_t src_h) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + /* setplane API takes shifted source rectangle values; unshift them */ > + src_x >>= 16; > + src_y >>= 16; > + src_w >>= 16; > + src_h >>= 16; > + > + if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) { > + DRM_DEBUG_KMS("Unsuitable framebuffer for primary plane\n"); > + return -EINVAL; > + } These aren't quite right for all gens. Actually I think the primary plane limits are genereally more relaxed than the sprite plane limits, so intel_framebuffer_init() should have already done the necessary checks, at least as far the stride is concerned. So I'd just drop the stride check. In the future I suppose we might need to add some extra checks here too, but for now I think we should fine w/o them. > + > + /* > + * Current hardware can't reposition the primary plane or scale it > + * (although this could change in the future). This means that we > + * don't actually need any of the destination (crtc) rectangle values, > + * or the source rectangle width/height; only the source x/y winds up > + * getting used for panning. Nevertheless, let's sanity check the > + * incoming values to make sure userspace didn't think it could scale > + * or reposition this plane. > + */ > + if (crtc_w != crtc->mode.hdisplay || crtc_h != crtc->mode.vdisplay || > + crtc_x != 0 || crtc_y != 0) { > + DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n"); > + return -EINVAL; > + } > + if (crtc_w != src_w || crtc_h != src_h) { > + DRM_DEBUG_KMS("Can't scale primary plane\n"); > + return -EINVAL; > + } I'd do the clipping anyway, and then check that the dst size matches the pipe src size post clipping. Otherwise the behaviour is rather inconsistent with the sprite planes. > + > + intel_pipe_set_base(crtc, src_x, src_y, fb); This can return an error. > + dev_priv->display.crtc_enable(crtc); Shouldn't enable the entire pipe, just the plane. > + > + return 0; > +} > + > +static int > +intel_primary_plane_disable(struct drm_plane *plane) > +{ > + struct drm_device *dev = plane->dev; > + drm_i915_private_t *dev_priv = dev->dev_private; > + > + if (!plane->fb) > + return 0; > + > + if (WARN_ON(!plane->crtc || plane->crtc->primary_plane != plane)) > + return -EINVAL; > + > + dev_priv->display.crtc_disable(plane->crtc); This should just disable the plane, not the entire pipe. > + > + return 0; > +} > + > +static void intel_primary_plane_destroy(struct drm_plane *plane) > +{ > + /* > + * Since primary planes are never put on the mode_config plane list, > + * this entry point should never be called. Primary plane cleanup > + * happens during CRTC destruction. > + */ > + BUG(); Just leave the func pointer as NULL, you get a backtrace either way. > +} > + > +static const struct drm_plane_funcs intel_primary_plane_funcs = { > + .update_plane = intel_primary_plane_setplane, > + .disable_plane = intel_primary_plane_disable, > + .destroy = intel_primary_plane_destroy, > +}; > + > static void intel_crtc_init(struct drm_device *dev, int pipe) > { > drm_i915_private_t *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc; > + struct drm_plane *primary_plane; > int i; > > intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL); > if (intel_crtc == NULL) > return; > > + primary_plane = kzalloc(sizeof(*primary_plane), GFP_KERNEL); > + if (primary_plane == NULL) { > + kfree(intel_crtc); > + return; > + } > + > drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs); > + drm_plane_set_primary(dev, primary_plane, &intel_crtc->base, > + &intel_primary_plane_funcs, NULL, 0); > > drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256); > for (i = 0; i < 256; i++) { > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9757010..d9a5cd5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8260,6 +8260,10 @@ static void intel_crtc_destroy(struct drm_crtc *crtc) intel_crtc_cursor_set(crtc, NULL, 0, 0, 0); + drm_plane_cleanup(crtc->primary_plane); + kfree(crtc->primary_plane); + crtc->primary_plane = NULL; + drm_crtc_cleanup(crtc); kfree(intel_crtc); @@ -10272,17 +10276,105 @@ static void intel_shared_dpll_init(struct drm_device *dev) BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS); } +static int +intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + /* setplane API takes shifted source rectangle values; unshift them */ + src_x >>= 16; + src_y >>= 16; + src_w >>= 16; + src_h >>= 16; + + if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) { + DRM_DEBUG_KMS("Unsuitable framebuffer for primary plane\n"); + return -EINVAL; + } + + /* + * Current hardware can't reposition the primary plane or scale it + * (although this could change in the future). This means that we + * don't actually need any of the destination (crtc) rectangle values, + * or the source rectangle width/height; only the source x/y winds up + * getting used for panning. Nevertheless, let's sanity check the + * incoming values to make sure userspace didn't think it could scale + * or reposition this plane. + */ + if (crtc_w != crtc->mode.hdisplay || crtc_h != crtc->mode.vdisplay || + crtc_x != 0 || crtc_y != 0) { + DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n"); + return -EINVAL; + } + if (crtc_w != src_w || crtc_h != src_h) { + DRM_DEBUG_KMS("Can't scale primary plane\n"); + return -EINVAL; + } + + intel_pipe_set_base(crtc, src_x, src_y, fb); + dev_priv->display.crtc_enable(crtc); + + return 0; +} + +static int +intel_primary_plane_disable(struct drm_plane *plane) +{ + struct drm_device *dev = plane->dev; + drm_i915_private_t *dev_priv = dev->dev_private; + + if (!plane->fb) + return 0; + + if (WARN_ON(!plane->crtc || plane->crtc->primary_plane != plane)) + return -EINVAL; + + dev_priv->display.crtc_disable(plane->crtc); + + return 0; +} + +static void intel_primary_plane_destroy(struct drm_plane *plane) +{ + /* + * Since primary planes are never put on the mode_config plane list, + * this entry point should never be called. Primary plane cleanup + * happens during CRTC destruction. + */ + BUG(); +} + +static const struct drm_plane_funcs intel_primary_plane_funcs = { + .update_plane = intel_primary_plane_setplane, + .disable_plane = intel_primary_plane_disable, + .destroy = intel_primary_plane_destroy, +}; + static void intel_crtc_init(struct drm_device *dev, int pipe) { drm_i915_private_t *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc; + struct drm_plane *primary_plane; int i; intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL); if (intel_crtc == NULL) return; + primary_plane = kzalloc(sizeof(*primary_plane), GFP_KERNEL); + if (primary_plane == NULL) { + kfree(intel_crtc); + return; + } + drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs); + drm_plane_set_primary(dev, primary_plane, &intel_crtc->base, + &intel_primary_plane_funcs, NULL, 0); drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256); for (i = 0; i < 256; i++) {
Create a primary plane at CRTC init and hook up handlers for the various operations that may be performed on it. The DRM core will only advertise the primary planes to clients that set the appropriate capability bit. Since we're limited to the legacy plane operations at the moment (SetPlane and such) this isn't terribly interesting yet; the plane update handler will perform an MMIO flip of the display plane and the disable handler will disable the CRTC. Once we migrate more of the plane and CRTC info over to properties in preparation for atomic/nuclear operations, primary planes will be more useful. Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)