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@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
