Re: [PATCH v2 2/5] drm/tegra: Add plane support

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

On Mon, Jan 14, 2013 at 06:03:44PM +0100, Lucas Stach wrote:
> Am Montag, den 14.01.2013, 17:05 +0100 schrieb Thierry Reding:
[...]
> > +	value = (WIN_A_ACT_REQ << index) | (WIN_A_UPDATE << index);
> > +	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
> This should be two separate writes to the register. I don't know how
> relevant this is on real HW, but the TRM states: "Restrictions: ACT_REQ
> cannot be programmed at the same time the corresponding "UPDATE" is
> programmed."
> 
> Better be safe than sorry and split it up.

It doesn't seem to make a difference, but I can split it up anyway.

[...]
> > +	/*
> > +	 * Disable blending and assume Window A is the bottom-most window,
> > +	 * Window C is the top-most window and Window B is in the middle.
> > +	 */
> I would like to see the root window using WIN_C, so we only loose the
> least capable plane (WIN_A: no filtering or YUV conversion) when using a
> plane for the hardware cursor. Maybe you can fold this in, otherwise
> I'll send a patch on top of this series.

On the other hand, doing so will loose a perfectly good video overlay
plane.

[...]
> > +	value = (WIN_A_ACT_REQ << index) | (WIN_A_UPDATE << index);
> > +	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
> Same comment as above.

Done. I'll fold a similar change into the .mode_set_base() patch and
will also add a patch that converts the remaining occurrences in
tegra_crtc_commit().

Thanks,
Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]