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

2013-01-18 Thread Mark Zhang 
On 01/15/2013 06:50 PM, Lucas Stach wrote:
> Am Dienstag, den 15.01.2013, 17:53 +0800 schrieb Mark Zhang:
>> On 01/15/2013 12:05 AM, Thierry Reding wrote:
>>> Add support for the B and C planes which support RGB and YUV pixel
>>> formats and can be used as overlays or hardware cursor.
>>
>> I think "hardware cursor" has specific meaning for Tegra(e.g: Tegra30
>> has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
>> to "hardware accelerated cursor"?
>>
> According to the TRM no Tegra has ARGB hardware cursor support, but only
> 2-color. So we talked about doing the hardware cursor by using a plane.
> If the TRM is wrong in this regard and we can get a ARGB cursor on Tegra
> 3 it would be nice to know.
> 

Lucas, yes, TRM says "Hardware cursor is supported for 32x32 or for
64x64 2-bpp cursor.", but just as you can see, we can set cursor's
foreground & background color by register "DC_DISP_CURSOR_FOREGROUND_0
" & "DC_DISP_CURSOR_BACKGROUND_0".

So I asked the expert in nvidia and here is the explanation of the
hardware cursor:

"each pixel in the cursor is encoded by 2 bits.
only 3 values are used per pixel: transparent, foreground, background.

when pixel is transparent - no pixel is displayed. (also known as a mask)
when pixel is foreground - color of pixel is 24-bit value in
DC_DISP_CURSOR_FOREGROUND_0.
when pixel is background - color of pixel is 24-bit value in
DC_DISP_CURSOR_BACKGROUND_0.

So I would still phrase it as a 2-bit cursor. It's a palette with 2
colors plus a 1-bit alpha. The palette entries are 24-bit."

Mark
>>>
>>> Signed-off-by: Thierry Reding 
>>> ---
>> [...]
>>> +};
>>> +
>>> +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
>>> +{
>>> +   unsigned int i;
>>> +   int err = 0;
>>> +
>>> +   for (i = 0; i < 2; i++) {
>>> +   struct tegra_plane *plane;
>>> +
>>> +   plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
>>> +   if (!plane)
>>> +   return -ENOMEM;
>>> +
>>> +   plane->index = i;
>>
>> I suggest to change this line to: "plane->index = i + 1;". This makes
>> the plane's index be consistent with Tegra's windows number. And also we
>> don't need to worry about passing "plane->index + 1" to some functions
>> which need to know which window is operating on.
>>
> Again, if we make WIN_C the root window, we can keep the plane index
> assignment as is and get rid of the "index + 1" passing.
> 
>

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

2013-01-17 Thread Mark Zhang 
On 01/15/2013 06:50 PM, Lucas Stach wrote:
 Am Dienstag, den 15.01.2013, 17:53 +0800 schrieb Mark Zhang:
 On 01/15/2013 12:05 AM, Thierry Reding wrote:
 Add support for the B and C planes which support RGB and YUV pixel
 formats and can be used as overlays or hardware cursor.

 I think hardware cursor has specific meaning for Tegra(e.g: Tegra30
 has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
 to hardware accelerated cursor?

 According to the TRM no Tegra has ARGB hardware cursor support, but only
 2-color. So we talked about doing the hardware cursor by using a plane.
 If the TRM is wrong in this regard and we can get a ARGB cursor on Tegra
 3 it would be nice to know.
 

Lucas, yes, TRM says Hardware cursor is supported for 32x32 or for
64x64 2-bpp cursor., but just as you can see, we can set cursor's
foreground  background color by register DC_DISP_CURSOR_FOREGROUND_0
  DC_DISP_CURSOR_BACKGROUND_0.

So I asked the expert in nvidia and here is the explanation of the
hardware cursor:

each pixel in the cursor is encoded by 2 bits.
only 3 values are used per pixel: transparent, foreground, background.

when pixel is transparent - no pixel is displayed. (also known as a mask)
when pixel is foreground - color of pixel is 24-bit value in
DC_DISP_CURSOR_FOREGROUND_0.
when pixel is background - color of pixel is 24-bit value in
DC_DISP_CURSOR_BACKGROUND_0.

So I would still phrase it as a 2-bit cursor. It's a palette with 2
colors plus a 1-bit alpha. The palette entries are 24-bit.

Mark

 Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
 ---
 [...]
 +};
 +
 +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
 +{
 +   unsigned int i;
 +   int err = 0;
 +
 +   for (i = 0; i  2; i++) {
 +   struct tegra_plane *plane;
 +
 +   plane = devm_kzalloc(drm-dev, sizeof(*plane), GFP_KERNEL);
 +   if (!plane)
 +   return -ENOMEM;
 +
 +   plane-index = i;

 I suggest to change this line to: plane-index = i + 1;. This makes
 the plane's index be consistent with Tegra's windows number. And also we
 don't need to worry about passing plane-index + 1 to some functions
 which need to know which window is operating on.

 Again, if we make WIN_C the root window, we can keep the plane index
 assignment as is and get rid of the index + 1 passing.
 
 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-01-15 Thread Mark Zhang 
On 01/15/2013 12:05 AM, Thierry Reding wrote:
> Add support for the B and C planes which support RGB and YUV pixel
> formats and can be used as overlays or hardware cursor.

I think "hardware cursor" has specific meaning for Tegra(e.g: Tegra30
has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
to "hardware accelerated cursor"?

> 
> Signed-off-by: Thierry Reding 
> ---
[...]
> +
> +static const uint32_t plane_formats[] = {
> + DRM_FORMAT_XRGB,
> + DRM_FORMAT_YUV422,

I haven't found something related with YUV format in this patch set. For
example, "tegra_dc_format" also doesn't take YUV into consideration. So
remove this line.

> +};
> +
> +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
> +{
> + unsigned int i;
> + int err = 0;
> +
> + for (i = 0; i < 2; i++) {
> + struct tegra_plane *plane;
> +
> + plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
> + if (!plane)
> + return -ENOMEM;
> +
> + plane->index = i;

I suggest to change this line to: "plane->index = i + 1;". This makes
the plane's index be consistent with Tegra's windows number. And also we
don't need to worry about passing "plane->index + 1" to some functions
which need to know which window is operating on.

> +
> + err = drm_plane_init(drm, >base, 1 << dc->pipe,
> +  _plane_funcs, plane_formats,
> +  ARRAY_SIZE(plane_formats), false);
> + if (err < 0)
> + return err;
> + }
> +
> + return 0;
> +}
> +
[...]
>  
> +int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
> +   const struct tegra_dc_window *window)
> +{
> + unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
> + unsigned long value;
> +
> + bpp = window->bits_per_pixel / 8;
> +
> + value = WINDOW_A_SELECT << index;
> + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
> +
> + tegra_dc_writel(dc, window->format, DC_WIN_COLOR_DEPTH);
> + tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP);
> +
> + value = V_POSITION(window->dst.y) | H_POSITION(window->dst.x);
> + tegra_dc_writel(dc, value, DC_WIN_POSITION);
> +
> + value = V_SIZE(window->dst.h) | H_SIZE(window->dst.w);
> + tegra_dc_writel(dc, value, DC_WIN_SIZE);
> +
> + h_offset = window->src.x * bpp;
> + v_offset = window->src.y;
> + h_size = window->src.w * bpp;
> + v_size = window->src.h;
> +
> + value = V_PRESCALED_SIZE(v_size) | H_PRESCALED_SIZE(h_size);
> + tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE);
> +
> + h_dda = compute_dda_inc(window->src.w, window->dst.w, false, bpp);
> + v_dda = compute_dda_inc(window->src.h, window->dst.h, true, bpp);
> +
> + value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda);
> + tegra_dc_writel(dc, value, DC_WIN_DDA_INC);
> +
> + h_dda = compute_initial_dda(window->src.x);
> + v_dda = compute_initial_dda(window->src.y);
> +

In current implementation, "compute_initial_dda" always returns zero. So
why we need it? Although according to TRM, typically we set H/V initial
dda to zero.

> + tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA);
> + tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA);
> +
> + tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE);
> + tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE);
> +
> + tegra_dc_writel(dc, window->base, DC_WINBUF_START_ADDR);
> + tegra_dc_writel(dc, window->stride, DC_WIN_LINE_STRIDE);
> + tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
> + tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
> +
> + value = WIN_ENABLE;
> +
> + if (bpp < 24)
> + value |= COLOR_EXPAND;
> +
> + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
> +
> + /*
> +  * 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.
> +  */
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_NOKEY);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_1WIN);
> +
> + switch (index) {
> + case 0:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> +
> + case 1:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> +
> + case 2:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> + }
> +
> + value = (WIN_A_ACT_REQ << index) |

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

2013-01-15 Thread Ville Syrjälä 
On Tue, Jan 15, 2013 at 05:53:03PM +0800, Mark Zhang wrote:
> On 01/15/2013 12:05 AM, Thierry Reding wrote:
> > Add support for the B and C planes which support RGB and YUV pixel
> > formats and can be used as overlays or hardware cursor.
> 
> I think "hardware cursor" has specific meaning for Tegra(e.g: Tegra30
> has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
> to "hardware accelerated cursor"?
> 
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> [...]
> > +
> > +static const uint32_t plane_formats[] = {
> > +   DRM_FORMAT_XRGB,
> > +   DRM_FORMAT_YUV422,
> 
> I haven't found something related with YUV format in this patch set. For
> example, "tegra_dc_format" also doesn't take YUV into consideration. So
> remove this line.

Also note that YUV422 is a planar format. And since it's not the most
common 4:2:2 format, my first guess would be that it's probably not
what you wanted. YUYV or UYVY is more likely the one you're after.

-- 
Ville Syrj?l?
Intel OTC

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

2013-01-15 Thread Thierry Reding 
On Tue, Jan 15, 2013 at 01:35:32PM +0200, Ville Syrj?l? wrote:
> On Tue, Jan 15, 2013 at 05:53:03PM +0800, Mark Zhang wrote:
> > On 01/15/2013 12:05 AM, Thierry Reding wrote:
> > > Add support for the B and C planes which support RGB and YUV pixel
> > > formats and can be used as overlays or hardware cursor.
> > 
> > I think "hardware cursor" has specific meaning for Tegra(e.g: Tegra30
> > has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
> > to "hardware accelerated cursor"?
> > 
> > > 
> > > Signed-off-by: Thierry Reding 
> > > ---
> > [...]
> > > +
> > > +static const uint32_t plane_formats[] = {
> > > + DRM_FORMAT_XRGB,
> > > + DRM_FORMAT_YUV422,
> > 
> > I haven't found something related with YUV format in this patch set. For
> > example, "tegra_dc_format" also doesn't take YUV into consideration. So
> > remove this line.
> 
> Also note that YUV422 is a planar format. And since it's not the most
> common 4:2:2 format, my first guess would be that it's probably not
> what you wanted. YUYV or UYVY is more likely the one you're after.

Yes, I copied it from the TRM, which has YUV422 listed as non-planar
format (it has YUV422, which is the planar variant). It isn't very
specific about which variant YUV422 really is, though.

As Mark said, the window setup code can't handle planar formats yet
and tegra_dc_format() doesn't convert between DRM and Tegra formats
other than 32-bit and 16-bit RGB either, so maybe I should just drop
it instead.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:

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

2013-01-15 Thread Thierry Reding 
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
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:

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

2013-01-15 Thread Lucas Stach 
Am Dienstag, den 15.01.2013, 17:53 +0800 schrieb Mark Zhang:
> On 01/15/2013 12:05 AM, Thierry Reding wrote:
> > Add support for the B and C planes which support RGB and YUV pixel
> > formats and can be used as overlays or hardware cursor.
> 
> I think "hardware cursor" has specific meaning for Tegra(e.g: Tegra30
> has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
> to "hardware accelerated cursor"?
> 
According to the TRM no Tegra has ARGB hardware cursor support, but only
2-color. So we talked about doing the hardware cursor by using a plane.
If the TRM is wrong in this regard and we can get a ARGB cursor on Tegra
3 it would be nice to know.

> > 
> > Signed-off-by: Thierry Reding 
> > ---
> [...]
> > +};
> > +
> > +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
> > +{
> > +   unsigned int i;
> > +   int err = 0;
> > +
> > +   for (i = 0; i < 2; i++) {
> > +   struct tegra_plane *plane;
> > +
> > +   plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
> > +   if (!plane)
> > +   return -ENOMEM;
> > +
> > +   plane->index = i;
> 
> I suggest to change this line to: "plane->index = i + 1;". This makes
> the plane's index be consistent with Tegra's windows number. And also we
> don't need to worry about passing "plane->index + 1" to some functions
> which need to know which window is operating on.
> 
Again, if we make WIN_C the root window, we can keep the plane index
assignment as is and get rid of the "index + 1" passing.

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

2013-01-15 Thread Mark Zhang 
On 01/15/2013 12:05 AM, Thierry Reding wrote:
 Add support for the B and C planes which support RGB and YUV pixel
 formats and can be used as overlays or hardware cursor.

I think hardware cursor has specific meaning for Tegra(e.g: Tegra30
has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
to hardware accelerated cursor?

 
 Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
 ---
[...]
 +
 +static const uint32_t plane_formats[] = {
 + DRM_FORMAT_XRGB,
 + DRM_FORMAT_YUV422,

I haven't found something related with YUV format in this patch set. For
example, tegra_dc_format also doesn't take YUV into consideration. So
remove this line.

 +};
 +
 +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
 +{
 + unsigned int i;
 + int err = 0;
 +
 + for (i = 0; i  2; i++) {
 + struct tegra_plane *plane;
 +
 + plane = devm_kzalloc(drm-dev, sizeof(*plane), GFP_KERNEL);
 + if (!plane)
 + return -ENOMEM;
 +
 + plane-index = i;

I suggest to change this line to: plane-index = i + 1;. This makes
the plane's index be consistent with Tegra's windows number. And also we
don't need to worry about passing plane-index + 1 to some functions
which need to know which window is operating on.

 +
 + err = drm_plane_init(drm, plane-base, 1  dc-pipe,
 +  tegra_plane_funcs, plane_formats,
 +  ARRAY_SIZE(plane_formats), false);
 + if (err  0)
 + return err;
 + }
 +
 + return 0;
 +}
 +
[...]
  
 +int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 +   const struct tegra_dc_window *window)
 +{
 + unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
 + unsigned long value;
 +
 + bpp = window-bits_per_pixel / 8;
 +
 + value = WINDOW_A_SELECT  index;
 + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 +
 + tegra_dc_writel(dc, window-format, DC_WIN_COLOR_DEPTH);
 + tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP);
 +
 + value = V_POSITION(window-dst.y) | H_POSITION(window-dst.x);
 + tegra_dc_writel(dc, value, DC_WIN_POSITION);
 +
 + value = V_SIZE(window-dst.h) | H_SIZE(window-dst.w);
 + tegra_dc_writel(dc, value, DC_WIN_SIZE);
 +
 + h_offset = window-src.x * bpp;
 + v_offset = window-src.y;
 + h_size = window-src.w * bpp;
 + v_size = window-src.h;
 +
 + value = V_PRESCALED_SIZE(v_size) | H_PRESCALED_SIZE(h_size);
 + tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE);
 +
 + h_dda = compute_dda_inc(window-src.w, window-dst.w, false, bpp);
 + v_dda = compute_dda_inc(window-src.h, window-dst.h, true, bpp);
 +
 + value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda);
 + tegra_dc_writel(dc, value, DC_WIN_DDA_INC);
 +
 + h_dda = compute_initial_dda(window-src.x);
 + v_dda = compute_initial_dda(window-src.y);
 +

In current implementation, compute_initial_dda always returns zero. So
why we need it? Although according to TRM, typically we set H/V initial
dda to zero.

 + tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA);
 + tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA);
 +
 + tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE);
 + tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE);
 +
 + tegra_dc_writel(dc, window-base, DC_WINBUF_START_ADDR);
 + tegra_dc_writel(dc, window-stride, DC_WIN_LINE_STRIDE);
 + tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
 + tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
 +
 + value = WIN_ENABLE;
 +
 + if (bpp  24)
 + value |= COLOR_EXPAND;
 +
 + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
 +
 + /*
 +  * 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.
 +  */
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_NOKEY);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_1WIN);
 +
 + switch (index) {
 + case 0:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 +
 + case 1:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 +
 + case 2:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 + }
 +
 + value = (WIN_A_ACT_REQ  index) | (WIN_A_UPDATE  index);
 + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
 +
 + return 0;
 +}
 +
 +unsigned

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

2013-01-15 Thread Lucas Stach 
Am Dienstag, den 15.01.2013, 17:53 +0800 schrieb Mark Zhang:
 On 01/15/2013 12:05 AM, Thierry Reding wrote:
  Add support for the B and C planes which support RGB and YUV pixel
  formats and can be used as overlays or hardware cursor.
 
 I think hardware cursor has specific meaning for Tegra(e.g: Tegra30
 has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
 to hardware accelerated cursor?
 
According to the TRM no Tegra has ARGB hardware cursor support, but only
2-color. So we talked about doing the hardware cursor by using a plane.
If the TRM is wrong in this regard and we can get a ARGB cursor on Tegra
3 it would be nice to know.

  
  Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
  ---
 [...]
  +};
  +
  +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
  +{
  +   unsigned int i;
  +   int err = 0;
  +
  +   for (i = 0; i  2; i++) {
  +   struct tegra_plane *plane;
  +
  +   plane = devm_kzalloc(drm-dev, sizeof(*plane), GFP_KERNEL);
  +   if (!plane)
  +   return -ENOMEM;
  +
  +   plane-index = i;
 
 I suggest to change this line to: plane-index = i + 1;. This makes
 the plane's index be consistent with Tegra's windows number. And also we
 don't need to worry about passing plane-index + 1 to some functions
 which need to know which window is operating on.
 
Again, if we make WIN_C the root window, we can keep the plane index
assignment as is and get rid of the index + 1 passing.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-01-15 Thread Thierry Reding 
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


pgp8daa5CyqrE.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-01-15 Thread Ville Syrjälä 
On Tue, Jan 15, 2013 at 05:53:03PM +0800, Mark Zhang wrote:
 On 01/15/2013 12:05 AM, Thierry Reding wrote:
  Add support for the B and C planes which support RGB and YUV pixel
  formats and can be used as overlays or hardware cursor.
 
 I think hardware cursor has specific meaning for Tegra(e.g: Tegra30
 has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
 to hardware accelerated cursor?
 
  
  Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
  ---
 [...]
  +
  +static const uint32_t plane_formats[] = {
  +   DRM_FORMAT_XRGB,
  +   DRM_FORMAT_YUV422,
 
 I haven't found something related with YUV format in this patch set. For
 example, tegra_dc_format also doesn't take YUV into consideration. So
 remove this line.

Also note that YUV422 is a planar format. And since it's not the most
common 4:2:2 format, my first guess would be that it's probably not
what you wanted. YUYV or UYVY is more likely the one you're after.

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-01-15 Thread Thierry Reding 
On Tue, Jan 15, 2013 at 01:35:32PM +0200, Ville Syrjälä wrote:
 On Tue, Jan 15, 2013 at 05:53:03PM +0800, Mark Zhang wrote:
  On 01/15/2013 12:05 AM, Thierry Reding wrote:
   Add support for the B and C planes which support RGB and YUV pixel
   formats and can be used as overlays or hardware cursor.
  
  I think hardware cursor has specific meaning for Tegra(e.g: Tegra30
  has a 32x32 24bpp or 64x64 2bpp hardware cursor). So you may change it
  to hardware accelerated cursor?
  
   
   Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
   ---
  [...]
   +
   +static const uint32_t plane_formats[] = {
   + DRM_FORMAT_XRGB,
   + DRM_FORMAT_YUV422,
  
  I haven't found something related with YUV format in this patch set. For
  example, tegra_dc_format also doesn't take YUV into consideration. So
  remove this line.
 
 Also note that YUV422 is a planar format. And since it's not the most
 common 4:2:2 format, my first guess would be that it's probably not
 what you wanted. YUYV or UYVY is more likely the one you're after.

Yes, I copied it from the TRM, which has YUV422 listed as non-planar
format (it has YUV422, which is the planar variant). It isn't very
specific about which variant YUV422 really is, though.

As Mark said, the window setup code can't handle planar formats yet
and tegra_dc_format() doesn't convert between DRM and Tegra formats
other than 32-bit and 16-bit RGB either, so maybe I should just drop
it instead.

Thierry


pgpoyoMwOLvN8.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-01-14 Thread Lucas Stach 
Am Montag, den 14.01.2013, 17:05 +0100 schrieb Thierry Reding:
> Add support for the B and C planes which support RGB and YUV pixel
> formats and can be used as overlays or hardware cursor.
> 
> Signed-off-by: Thierry Reding 
> ---
[...]
> +static int tegra_plane_disable(struct drm_plane *plane)
> +{
> + struct tegra_dc *dc = to_tegra_dc(plane->crtc);
> + struct tegra_plane *p = to_tegra_plane(plane);
> + unsigned int index = p->index + 1;
> + unsigned long value;
> +
> + value = WINDOW_A_SELECT << index;
> + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
> +
> + value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
> + value &= ~WIN_ENABLE;
> + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
> +
> + 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.
[...]
> +int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
> +   const struct tegra_dc_window *window)
> +{
> + unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
> + unsigned long value;
> +
> + bpp = window->bits_per_pixel / 8;
> +
> + value = WINDOW_A_SELECT << index;
> + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
> +
> + tegra_dc_writel(dc, window->format, DC_WIN_COLOR_DEPTH);
> + tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP);
> +
> + value = V_POSITION(window->dst.y) | H_POSITION(window->dst.x);
> + tegra_dc_writel(dc, value, DC_WIN_POSITION);
> +
> + value = V_SIZE(window->dst.h) | H_SIZE(window->dst.w);
> + tegra_dc_writel(dc, value, DC_WIN_SIZE);
> +
> + h_offset = window->src.x * bpp;
> + v_offset = window->src.y;
> + h_size = window->src.w * bpp;
> + v_size = window->src.h;
> +
> + value = V_PRESCALED_SIZE(v_size) | H_PRESCALED_SIZE(h_size);
> + tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE);
> +
> + h_dda = compute_dda_inc(window->src.w, window->dst.w, false, bpp);
> + v_dda = compute_dda_inc(window->src.h, window->dst.h, true, bpp);
> +
> + value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda);
> + tegra_dc_writel(dc, value, DC_WIN_DDA_INC);
> +
> + h_dda = compute_initial_dda(window->src.x);
> + v_dda = compute_initial_dda(window->src.y);
> +
> + tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA);
> + tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA);
> +
> + tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE);
> + tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE);
> +
> + tegra_dc_writel(dc, window->base, DC_WINBUF_START_ADDR);
> + tegra_dc_writel(dc, window->stride, DC_WIN_LINE_STRIDE);
> + tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
> + tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
> +
> + value = WIN_ENABLE;
> +
> + if (bpp < 24)
> + value |= COLOR_EXPAND;
> +
> + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
> +
> + /*
> +  * 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.

> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_NOKEY);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_1WIN);
> +
> + switch (index) {
> + case 0:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> +
> + case 1:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> +
> + case 2:
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
> + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
> + break;
> + }
> +
> + value = (WIN_A_ACT_REQ << index) | (WIN_A_UPDATE << index);
> + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
Same comment as above.
> +
> + return 0;
> +}
> +
[...]

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

2013-01-14 Thread Thierry Reding 
Add support for the B and C planes which support RGB and YUV pixel
formats and can be used as overlays or hardware cursor.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/dc.c  | 321 +++-
 drivers/gpu/drm/tegra/dc.h  |   2 +-
 drivers/gpu/drm/tegra/drm.h |  29 
 3 files changed, 259 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 656b2e3..157e962 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -18,19 +18,104 @@
 #include "drm.h"
 #include "dc.h"

-struct tegra_dc_window {
-   fixed20_12 x;
-   fixed20_12 y;
-   fixed20_12 w;
-   fixed20_12 h;
-   unsigned int outx;
-   unsigned int outy;
-   unsigned int outw;
-   unsigned int outh;
-   unsigned int stride;
-   unsigned int fmt;
+struct tegra_plane {
+   struct drm_plane base;
+   unsigned int index;
 };

+static inline struct tegra_plane *to_tegra_plane(struct drm_plane *plane)
+{
+   return container_of(plane, struct tegra_plane, base);
+}
+
+static int tegra_plane_update(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)
+{
+   unsigned long base = tegra_framebuffer_base(fb);
+   struct tegra_plane *p = to_tegra_plane(plane);
+   struct tegra_dc *dc = to_tegra_dc(crtc);
+   struct tegra_dc_window window;
+
+   memset(, 0, sizeof(window));
+   window.src.x = src_x >> 16;
+   window.src.y = src_y >> 16;
+   window.src.w = src_w >> 16;
+   window.src.h = src_h >> 16;
+   window.dst.x = crtc_x;
+   window.dst.y = crtc_y;
+   window.dst.w = crtc_w;
+   window.dst.h = crtc_h;
+   window.format = tegra_dc_format(fb->pixel_format);
+   window.bits_per_pixel = fb->bits_per_pixel;
+   window.stride = fb->pitches[0];
+   window.base = base;
+
+   return tegra_dc_setup_window(dc, p->index + 1, );
+}
+
+static int tegra_plane_disable(struct drm_plane *plane)
+{
+   struct tegra_dc *dc = to_tegra_dc(plane->crtc);
+   struct tegra_plane *p = to_tegra_plane(plane);
+   unsigned int index = p->index + 1;
+   unsigned long value;
+
+   value = WINDOW_A_SELECT << index;
+   tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
+
+   value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
+   value &= ~WIN_ENABLE;
+   tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
+
+   value = (WIN_A_ACT_REQ << index) | (WIN_A_UPDATE << index);
+   tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
+
+   return 0;
+}
+
+static void tegra_plane_destroy(struct drm_plane *plane)
+{
+   drm_plane_cleanup(plane);
+}
+
+static const struct drm_plane_funcs tegra_plane_funcs = {
+   .update_plane = tegra_plane_update,
+   .disable_plane = tegra_plane_disable,
+   .destroy = tegra_plane_destroy,
+};
+
+static const uint32_t plane_formats[] = {
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_YUV422,
+};
+
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
+{
+   unsigned int i;
+   int err = 0;
+
+   for (i = 0; i < 2; i++) {
+   struct tegra_plane *plane;
+
+   plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
+   if (!plane)
+   return -ENOMEM;
+
+   plane->index = i;
+
+   err = drm_plane_init(drm, >base, 1 << dc->pipe,
+_plane_funcs, plane_formats,
+ARRAY_SIZE(plane_formats), false);
+   if (err < 0)
+   return err;
+   }
+
+   return 0;
+}
+
 static const struct drm_crtc_funcs tegra_crtc_funcs = {
.set_config = drm_crtc_helper_set_config,
.destroy = drm_crtc_cleanup,
@@ -47,10 +132,11 @@ static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }

-static inline u32 compute_dda_inc(fixed20_12 inf, unsigned int out, bool v,
+static inline u32 compute_dda_inc(unsigned int in, unsigned int out, bool v,
  unsigned int bpp)
 {
fixed20_12 outf = dfixed_init(out);
+   fixed20_12 inf = dfixed_init(in);
u32 dda_inc;
int max;

@@ -80,9 +166,10 @@ static inline u32 compute_dda_inc(fixed20_12 inf, unsigned 
int out, bool v,
return dda_inc;
 }

-static inline u32 compute_initial_dda(fixed20_12 in)
+static inline u32 compute_initial_dda(unsigned int in)
 {
-   return dfixed_frac(in);
+   fixed20_12 inf = dfixed_init(in);
+   return dfixed_frac(inf);
 }

 static int tegra_dc_set_timings(struct tegra_dc *dc,
@@ -153,6 +240,111 @@ static int tegra_crtc_setup_clk(struct

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

2013-01-14 Thread Thierry Reding 
Add support for the B and C planes which support RGB and YUV pixel
formats and can be used as overlays or hardware cursor.

Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
---
 drivers/gpu/drm/tegra/dc.c  | 321 +++-
 drivers/gpu/drm/tegra/dc.h  |   2 +-
 drivers/gpu/drm/tegra/drm.h |  29 
 3 files changed, 259 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 656b2e3..157e962 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -18,19 +18,104 @@
 #include drm.h
 #include dc.h
 
-struct tegra_dc_window {
-   fixed20_12 x;
-   fixed20_12 y;
-   fixed20_12 w;
-   fixed20_12 h;
-   unsigned int outx;
-   unsigned int outy;
-   unsigned int outw;
-   unsigned int outh;
-   unsigned int stride;
-   unsigned int fmt;
+struct tegra_plane {
+   struct drm_plane base;
+   unsigned int index;
 };
 
+static inline struct tegra_plane *to_tegra_plane(struct drm_plane *plane)
+{
+   return container_of(plane, struct tegra_plane, base);
+}
+
+static int tegra_plane_update(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)
+{
+   unsigned long base = tegra_framebuffer_base(fb);
+   struct tegra_plane *p = to_tegra_plane(plane);
+   struct tegra_dc *dc = to_tegra_dc(crtc);
+   struct tegra_dc_window window;
+
+   memset(window, 0, sizeof(window));
+   window.src.x = src_x  16;
+   window.src.y = src_y  16;
+   window.src.w = src_w  16;
+   window.src.h = src_h  16;
+   window.dst.x = crtc_x;
+   window.dst.y = crtc_y;
+   window.dst.w = crtc_w;
+   window.dst.h = crtc_h;
+   window.format = tegra_dc_format(fb-pixel_format);
+   window.bits_per_pixel = fb-bits_per_pixel;
+   window.stride = fb-pitches[0];
+   window.base = base;
+
+   return tegra_dc_setup_window(dc, p-index + 1, window);
+}
+
+static int tegra_plane_disable(struct drm_plane *plane)
+{
+   struct tegra_dc *dc = to_tegra_dc(plane-crtc);
+   struct tegra_plane *p = to_tegra_plane(plane);
+   unsigned int index = p-index + 1;
+   unsigned long value;
+
+   value = WINDOW_A_SELECT  index;
+   tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
+
+   value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
+   value = ~WIN_ENABLE;
+   tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
+
+   value = (WIN_A_ACT_REQ  index) | (WIN_A_UPDATE  index);
+   tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
+
+   return 0;
+}
+
+static void tegra_plane_destroy(struct drm_plane *plane)
+{
+   drm_plane_cleanup(plane);
+}
+
+static const struct drm_plane_funcs tegra_plane_funcs = {
+   .update_plane = tegra_plane_update,
+   .disable_plane = tegra_plane_disable,
+   .destroy = tegra_plane_destroy,
+};
+
+static const uint32_t plane_formats[] = {
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_YUV422,
+};
+
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
+{
+   unsigned int i;
+   int err = 0;
+
+   for (i = 0; i  2; i++) {
+   struct tegra_plane *plane;
+
+   plane = devm_kzalloc(drm-dev, sizeof(*plane), GFP_KERNEL);
+   if (!plane)
+   return -ENOMEM;
+
+   plane-index = i;
+
+   err = drm_plane_init(drm, plane-base, 1  dc-pipe,
+tegra_plane_funcs, plane_formats,
+ARRAY_SIZE(plane_formats), false);
+   if (err  0)
+   return err;
+   }
+
+   return 0;
+}
+
 static const struct drm_crtc_funcs tegra_crtc_funcs = {
.set_config = drm_crtc_helper_set_config,
.destroy = drm_crtc_cleanup,
@@ -47,10 +132,11 @@ static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }
 
-static inline u32 compute_dda_inc(fixed20_12 inf, unsigned int out, bool v,
+static inline u32 compute_dda_inc(unsigned int in, unsigned int out, bool v,
  unsigned int bpp)
 {
fixed20_12 outf = dfixed_init(out);
+   fixed20_12 inf = dfixed_init(in);
u32 dda_inc;
int max;
 
@@ -80,9 +166,10 @@ static inline u32 compute_dda_inc(fixed20_12 inf, unsigned 
int out, bool v,
return dda_inc;
 }
 
-static inline u32 compute_initial_dda(fixed20_12 in)
+static inline u32 compute_initial_dda(unsigned int in)
 {
-   return dfixed_frac(in);
+   fixed20_12 inf = dfixed_init(in);
+   return dfixed_frac(inf);
 }
 
 static int tegra_dc_set_timings(struct tegra_dc *dc,
@@ -153,6 +240,111 @@ static int

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

2013-01-14 Thread Lucas Stach 
Am Montag, den 14.01.2013, 17:05 +0100 schrieb Thierry Reding:
 Add support for the B and C planes which support RGB and YUV pixel
 formats and can be used as overlays or hardware cursor.
 
 Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
 ---
[...]
 +static int tegra_plane_disable(struct drm_plane *plane)
 +{
 + struct tegra_dc *dc = to_tegra_dc(plane-crtc);
 + struct tegra_plane *p = to_tegra_plane(plane);
 + unsigned int index = p-index + 1;
 + unsigned long value;
 +
 + value = WINDOW_A_SELECT  index;
 + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 +
 + value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
 + value = ~WIN_ENABLE;
 + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
 +
 + 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.
[...]
 +int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 +   const struct tegra_dc_window *window)
 +{
 + unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
 + unsigned long value;
 +
 + bpp = window-bits_per_pixel / 8;
 +
 + value = WINDOW_A_SELECT  index;
 + tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 +
 + tegra_dc_writel(dc, window-format, DC_WIN_COLOR_DEPTH);
 + tegra_dc_writel(dc, 0, DC_WIN_BYTE_SWAP);
 +
 + value = V_POSITION(window-dst.y) | H_POSITION(window-dst.x);
 + tegra_dc_writel(dc, value, DC_WIN_POSITION);
 +
 + value = V_SIZE(window-dst.h) | H_SIZE(window-dst.w);
 + tegra_dc_writel(dc, value, DC_WIN_SIZE);
 +
 + h_offset = window-src.x * bpp;
 + v_offset = window-src.y;
 + h_size = window-src.w * bpp;
 + v_size = window-src.h;
 +
 + value = V_PRESCALED_SIZE(v_size) | H_PRESCALED_SIZE(h_size);
 + tegra_dc_writel(dc, value, DC_WIN_PRESCALED_SIZE);
 +
 + h_dda = compute_dda_inc(window-src.w, window-dst.w, false, bpp);
 + v_dda = compute_dda_inc(window-src.h, window-dst.h, true, bpp);
 +
 + value = V_DDA_INC(v_dda) | H_DDA_INC(h_dda);
 + tegra_dc_writel(dc, value, DC_WIN_DDA_INC);
 +
 + h_dda = compute_initial_dda(window-src.x);
 + v_dda = compute_initial_dda(window-src.y);
 +
 + tegra_dc_writel(dc, h_dda, DC_WIN_H_INITIAL_DDA);
 + tegra_dc_writel(dc, v_dda, DC_WIN_V_INITIAL_DDA);
 +
 + tegra_dc_writel(dc, 0, DC_WIN_UV_BUF_STRIDE);
 + tegra_dc_writel(dc, 0, DC_WIN_BUF_STRIDE);
 +
 + tegra_dc_writel(dc, window-base, DC_WINBUF_START_ADDR);
 + tegra_dc_writel(dc, window-stride, DC_WIN_LINE_STRIDE);
 + tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
 + tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
 +
 + value = WIN_ENABLE;
 +
 + if (bpp  24)
 + value |= COLOR_EXPAND;
 +
 + tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
 +
 + /*
 +  * 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.

 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_NOKEY);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_1WIN);
 +
 + switch (index) {
 + case 0:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 +
 + case 1:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 +
 + case 2:
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_X);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_2WIN_Y);
 + tegra_dc_writel(dc, 0x00, DC_WIN_BLEND_3WIN_XY);
 + break;
 + }
 +
 + value = (WIN_A_ACT_REQ  index) | (WIN_A_UPDATE  index);
 + tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
Same comment as above.
 +
 + return 0;
 +}
 +
[...]

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel