Re: [PATCH 2/4] drm/plane-helper: Add drm_primary_helper_check_update() (v2)

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, May 16, 2014 at 10:51:44AM +0800, Lee, Chon Ming wrote:
...
> > +int drm_primary_helper_check_update(struct drm_plane *plane,
> > +				    struct drm_crtc *crtc,
> > +				    struct drm_framebuffer *fb,
> > +				    struct drm_rect *src,
> > +				    struct drm_rect *dest,
> > +				    const struct drm_rect *clip,
> > +				    int min_scale,
> > +				    int max_scale,
> > +				    bool can_position,
> > +				    bool can_update_disabled,
> > +				    bool *visible)
> > +{
> > +	int hscale, vscale;
> > +
> > +	if (!crtc->enabled && !can_update_disabled) {
> > +		DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Check scaling */
> > +	hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
> > +	vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
> > +	if (hscale < 0 || vscale < 0) {
> > +		DRM_DEBUG_KMS("Invalid scaling of primary plane\n");
> > +		return -ERANGE;
> > +	}
> > +
> > +	*visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
> > +	if (!visible)
> > +		/*
> > +		 * Primary plane isn't visible; some drivers can handle this
> > +		 * so we just return success here.  Drivers that can't
> > +		 * (including those that use the primary plane helper's
> > +		 * update function) will return an error from their
> > +		 * update_plane handler.
> > +		 */
> > +		return 0;
> > +
> > +	if (!can_position && !drm_rect_equals(dest, clip)) {
> > +		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
> > +		return -EINVAL;
> > +	}
> 
> Cherryview display allow the primary plane to be position at any location
> similiar to sprite plane for certain port. So, this shouldn't need to check here.  
> 
> And the width/height doesn't need to cover the whole screen.

Right, but this is a general helper function in the DRM core that is
meant to be usable on all hardware and on all vendors' drivers
(including the simple primary planes that are automatically created by
helper functions for drivers that don't provide their own primary plane
support).  The goal here is to centralize the common parameter checking
in one place so that all drivers don't have to duplicate the same set of
checks, but give a little bit of flexibility so that drivers for more
feature-rich hardware can relax some of the restrictions that their
hardware can actually handle (such as Cherryview being able to do
primary plane windowing as you pointed out).

It's true that the i915-specific implementation could be further
extended to pass true for the 'can_position' parameter when running on
Cherrytrail and then program the hardware accordingly, but that's really
an extra feature beyond what I'm adding here; we'd want to add that as a
follow-on patch later and come up with a whole extra set of tests to
exercise it.  I'd rather focus on getting this general i915 support here
merged first, then go back and start enabling new hardware
functionalities like that on the newer platforms that can handle it.

I'll add Cherryview primary plane windowing to my TODO list for future
work if nobody beats me to it (I think some of the guys in VPG may
already be looking into this).


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795