From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH v3 4/4] drm/i915: Implement proper clipping for video
	sprites
Date: Tue, 16 Apr 2013 17:20:15 +0300
Message-ID: <20130416142015.GK4469@intel.com>
References: <1366109242-15950-1-git-send-email-ville.syrjala@linux.intel.com>
	<1366109242-15950-5-git-send-email-ville.syrjala@linux.intel.com>
	<20130416133702.GA13387@cantiga.alporthouse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Content-Disposition: inline
In-Reply-To: <20130416133702.GA13387@cantiga.alporthouse.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Chris Wilson <chris@chris-wilson.co.uk>, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

On Tue, Apr 16, 2013 at 02:37:24PM +0100, Chris Wilson wrote:
> On Tue, Apr 16, 2013 at 01:47:22PM +0300, ville.syrjala@linux.intel.com w=
rote:
> > From: Ville Syrj=E4l=E4 <ville.syrjala@linux.intel.com>
> > =

> > Properly clip the source when the destination gets clipped
> > by the pipe dimensions.
> > =

> > Sadly the video sprite hardware is rather limited so it can't do proper
> > sub-pixel postitioning. Resort to truncating the source coordinates to
> > (macro)pixel boundary.
> > =

> > The scaling checks are done using the relaxed drm_region functions.
> > That means that the src/dst regions are reduced in size in order to keep
> > the scaling factor within the limits.
> > =

> > Also do some additional checking against various hardware limits.
> > =

> > v2: Truncate src coords instead of rounding to avoid increasing src
> >     viewport size, and adapt to changes in drm_calc_{h,v}scale().
> > v3: Adapt to drm_region->drm_rect rename. Fix misaligned crtc_w for
> >     packed YUV formats when scaling isn't supported.
> > =

> > Signed-off-by: Ville Syrj=E4l=E4 <ville.syrjala@linux.intel.com>
> =

> Skipping to the end, use of drm_rect looks good.
> =

> The one ugly that stood out is:
> =

> >  	/*
> >  	 * If the sprite is completely covering the primary plane,
> >  	 * we can disable the primary and save power.
> >  	 */
> > -	if ((crtc_x =3D=3D 0) && (crtc_y =3D=3D 0) &&
> > +	if (visible &&
> > +	    (crtc_x =3D=3D 0) && (crtc_y =3D=3D 0) &&
> >  	    (crtc_w =3D=3D primary_w) && (crtc_h =3D=3D primary_h))
> >  		disable_primary =3D true;
> =

> which would be
>   disable_primary =3D drm_rect_equals(&dst, &clip);
>   BUG_ON(disable_primary && !visible);
> with a little bit of massaging of crtc/dst.

Yeah, looks nicer. I'll add drm_rect_equals() to our repertoire.

-- =

Ville Syrj=E4l=E4
Intel OTC