Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Dmitry Osipenko
On 21.12.2017 17:10, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
>> On 21.12.2017 01:23, Dmitry Osipenko wrote:
>>> On 21.12.2017 01:02, Thierry Reding wrote:
 On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 23:16, Thierry Reding wrote:
>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>> On 20.12.2017 21:01, Thierry Reding wrote:
 On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format 
> if
> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore 
> with
> both modesetting and opentegra drivers. On older Tegra's each plane 
> has
> a blending configuration which should be used to enable / disable 
> alpha
> blending and right now the blending configs are hardcoded to disabled
> alpha blending. In order to support alpha formats properly, planes
> blending configuration must be adjusted, until then the alpha formats
> are equal to non-alpha.
>
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/tegra/dc.c| 29 ++---
>  drivers/gpu/drm/tegra/dc.h|  1 +
>  drivers/gpu/drm/tegra/fb.c| 13 -
>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
>  drivers/gpu/drm/tegra/plane.c | 22 +-
>  drivers/gpu/drm/tegra/plane.h |  2 +-
>  6 files changed, 39 insertions(+), 31 deletions(-)

 This kept bugging me, so I spent some time looking at the blending
 programming. I came up with the attached patch which seems to work
 for all scenarios and is fairly similar to your patch. It has the
 added benefit that we can keep support for more formats.

 Any comments?

 Thierry
 --- >8 ---
 From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
 From: Thierry Reding 
 Date: Wed, 20 Dec 2017 09:39:14 +0100
 Subject: [PATCH] drm/tegra: dc: Implement legacy blending

 This implements alpha blending on legacy display controllers (Tegra20,
 Tegra30 and Tegra114). While it's theoretically possible to support the
 zpos property to enable userspace to specify the Z-order of each plane
 individually, this is not currently supported and the same fixed Z-
 order as previously defined is used.
>>>
>>> Perhaps one variant of implementing zpos could be by making overlays 
>>> 'virtual',
>>> so each virtual overlay will be backed by the real HW plane and we 
>>> could swap
>>> the HW planes of the virtual overlays, emulating zpos.
>>>
 Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") 
 since
 the opaque formats are now supported.

 Reported-by: Dmitry Osipenko 
 Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
 Signed-off-by: Thierry Reding 
 ---
  drivers/gpu/drm/tegra/dc.c| 74 
 ++-
  drivers/gpu/drm/tegra/dc.h| 13 
  drivers/gpu/drm/tegra/fb.c| 12 ---
  drivers/gpu/drm/tegra/plane.c | 41 
  drivers/gpu/drm/tegra/plane.h |  3 ++
  5 files changed, 116 insertions(+), 27 deletions(-)

 diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
 index bc65c314e00f..07c687d7f615 100644
 --- a/drivers/gpu/drm/tegra/dc.c
 +++ b/drivers/gpu/drm/tegra/dc.c
 @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned 
 int in)
return dfixed_frac(inf);
  }
  
 -static void tegra_plane_setup_blending_legacy(struct tegra_plane 
 *plane)
 +static void
 +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
 +const struct tegra_dc_window *window)
  {
 +  u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
 +   BLEND_COLOR_KEY_NONE;
 +  u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
 +   BLEND_COLOR_KEY_NONE;
 +  u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
 +
 +  /* enable alpha blending if window->alpha */
 +  if (window->alpha) {
 +  background |= BLEND_CONTROL_DEPENDENT;
 +

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Thierry Reding
On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
> On 21.12.2017 01:23, Dmitry Osipenko wrote:
> > On 21.12.2017 01:02, Thierry Reding wrote:
> >> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> >>> On 20.12.2017 23:16, Thierry Reding wrote:
>  On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> > On 20.12.2017 21:01, Thierry Reding wrote:
> >> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
> >>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format 
> >>> if
> >>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore 
> >>> with
> >>> both modesetting and opentegra drivers. On older Tegra's each plane 
> >>> has
> >>> a blending configuration which should be used to enable / disable 
> >>> alpha
> >>> blending and right now the blending configs are hardcoded to disabled
> >>> alpha blending. In order to support alpha formats properly, planes
> >>> blending configuration must be adjusted, until then the alpha formats
> >>> are equal to non-alpha.
> >>>
> >>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >>> Signed-off-by: Dmitry Osipenko 
> >>> ---
> >>>  drivers/gpu/drm/tegra/dc.c| 29 ++---
> >>>  drivers/gpu/drm/tegra/dc.h|  1 +
> >>>  drivers/gpu/drm/tegra/fb.c| 13 -
> >>>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
> >>>  drivers/gpu/drm/tegra/plane.c | 22 +-
> >>>  drivers/gpu/drm/tegra/plane.h |  2 +-
> >>>  6 files changed, 39 insertions(+), 31 deletions(-)
> >>
> >> This kept bugging me, so I spent some time looking at the blending
> >> programming. I came up with the attached patch which seems to work
> >> for all scenarios and is fairly similar to your patch. It has the
> >> added benefit that we can keep support for more formats.
> >>
> >> Any comments?
> >>
> >> Thierry
> >> --- >8 ---
> >> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
> >> From: Thierry Reding 
> >> Date: Wed, 20 Dec 2017 09:39:14 +0100
> >> Subject: [PATCH] drm/tegra: dc: Implement legacy blending
> >>
> >> This implements alpha blending on legacy display controllers (Tegra20,
> >> Tegra30 and Tegra114). While it's theoretically possible to support the
> >> zpos property to enable userspace to specify the Z-order of each plane
> >> individually, this is not currently supported and the same fixed Z-
> >> order as previously defined is used.
> >
> > Perhaps one variant of implementing zpos could be by making overlays 
> > 'virtual',
> > so each virtual overlay will be backed by the real HW plane and we 
> > could swap
> > the HW planes of the virtual overlays, emulating zpos.
> >
> >> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") 
> >> since
> >> the opaque formats are now supported.
> >>
> >> Reported-by: Dmitry Osipenko 
> >> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >> Signed-off-by: Thierry Reding 
> >> ---
> >>  drivers/gpu/drm/tegra/dc.c| 74 
> >> ++-
> >>  drivers/gpu/drm/tegra/dc.h| 13 
> >>  drivers/gpu/drm/tegra/fb.c| 12 ---
> >>  drivers/gpu/drm/tegra/plane.c | 41 
> >>  drivers/gpu/drm/tegra/plane.h |  3 ++
> >>  5 files changed, 116 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >> index bc65c314e00f..07c687d7f615 100644
> >> --- a/drivers/gpu/drm/tegra/dc.c
> >> +++ b/drivers/gpu/drm/tegra/dc.c
> >> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned 
> >> int in)
> >>return dfixed_frac(inf);
> >>  }
> >>  
> >> -static void tegra_plane_setup_blending_legacy(struct tegra_plane 
> >> *plane)
> >> +static void
> >> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
> >> +const struct tegra_dc_window *window)
> >>  {
> >> +  u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
> >> +   BLEND_COLOR_KEY_NONE;
> >> +  u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
> >> +   BLEND_COLOR_KEY_NONE;
> >> +  u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
> >> +
> >> +  /* enable alpha blending if window->alpha */
> >> +  if (window->alpha) {
> >> +  background |= BLEND_CONTROL_DEPENDENT;
> >> +  foreground |= BLEND_CONTROL_ALPHA;

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Dmitry Osipenko
On 21.12.2017 01:23, Dmitry Osipenko wrote:
> On 21.12.2017 01:02, Thierry Reding wrote:
>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>>> On 20.12.2017 23:16, Thierry Reding wrote:
 On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 21:01, Thierry Reding wrote:
>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
>>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if
>>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with
>>> both modesetting and opentegra drivers. On older Tegra's each plane has
>>> a blending configuration which should be used to enable / disable alpha
>>> blending and right now the blending configs are hardcoded to disabled
>>> alpha blending. In order to support alpha formats properly, planes
>>> blending configuration must be adjusted, until then the alpha formats
>>> are equal to non-alpha.
>>>
>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>> Signed-off-by: Dmitry Osipenko 
>>> ---
>>>  drivers/gpu/drm/tegra/dc.c| 29 ++---
>>>  drivers/gpu/drm/tegra/dc.h|  1 +
>>>  drivers/gpu/drm/tegra/fb.c| 13 -
>>>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
>>>  drivers/gpu/drm/tegra/plane.c | 22 +-
>>>  drivers/gpu/drm/tegra/plane.h |  2 +-
>>>  6 files changed, 39 insertions(+), 31 deletions(-)
>>
>> This kept bugging me, so I spent some time looking at the blending
>> programming. I came up with the attached patch which seems to work
>> for all scenarios and is fairly similar to your patch. It has the
>> added benefit that we can keep support for more formats.
>>
>> Any comments?
>>
>> Thierry
>> --- >8 ---
>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
>> From: Thierry Reding 
>> Date: Wed, 20 Dec 2017 09:39:14 +0100
>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending
>>
>> This implements alpha blending on legacy display controllers (Tegra20,
>> Tegra30 and Tegra114). While it's theoretically possible to support the
>> zpos property to enable userspace to specify the Z-order of each plane
>> individually, this is not currently supported and the same fixed Z-
>> order as previously defined is used.
>
> Perhaps one variant of implementing zpos could be by making overlays 
> 'virtual',
> so each virtual overlay will be backed by the real HW plane and we could 
> swap
> the HW planes of the virtual overlays, emulating zpos.
>
>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>> the opaque formats are now supported.
>>
>> Reported-by: Dmitry Osipenko 
>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>> Signed-off-by: Thierry Reding 
>> ---
>>  drivers/gpu/drm/tegra/dc.c| 74 
>> ++-
>>  drivers/gpu/drm/tegra/dc.h| 13 
>>  drivers/gpu/drm/tegra/fb.c| 12 ---
>>  drivers/gpu/drm/tegra/plane.c | 41 
>>  drivers/gpu/drm/tegra/plane.h |  3 ++
>>  5 files changed, 116 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index bc65c314e00f..07c687d7f615 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int 
>> in)
>>  return dfixed_frac(inf);
>>  }
>>  
>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
>> +static void
>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
>> +  const struct tegra_dc_window *window)
>>  {
>> +u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
>> + BLEND_COLOR_KEY_NONE;
>> +u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
>> + BLEND_COLOR_KEY_NONE;
>> +u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
>> +
>> +/* enable alpha blending if window->alpha */
>> +if (window->alpha) {
>> +background |= BLEND_CONTROL_DEPENDENT;
>> +foreground |= BLEND_CONTROL_ALPHA;
>> +}
>
> I think dependent weight means that window doesn't have alpha 
> transparency. So
> we should set the dependent_weight mode for opaque formats and 
> alpha_weight for
> formats with alpha channel.

 According to the TRM, dependent weight 

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Dmitry Osipenko
On 21.12.2017 01:02, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 23:16, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
 On 20.12.2017 21:01, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if
>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with
>> both modesetting and opentegra drivers. On older Tegra's each plane has
>> a blending configuration which should be used to enable / disable alpha
>> blending and right now the blending configs are hardcoded to disabled
>> alpha blending. In order to support alpha formats properly, planes
>> blending configuration must be adjusted, until then the alpha formats
>> are equal to non-alpha.
>>
>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/gpu/drm/tegra/dc.c| 29 ++---
>>  drivers/gpu/drm/tegra/dc.h|  1 +
>>  drivers/gpu/drm/tegra/fb.c| 13 -
>>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
>>  drivers/gpu/drm/tegra/plane.c | 22 +-
>>  drivers/gpu/drm/tegra/plane.h |  2 +-
>>  6 files changed, 39 insertions(+), 31 deletions(-)
>
> This kept bugging me, so I spent some time looking at the blending
> programming. I came up with the attached patch which seems to work
> for all scenarios and is fairly similar to your patch. It has the
> added benefit that we can keep support for more formats.
>
> Any comments?
>
> Thierry
> --- >8 ---
> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
> From: Thierry Reding 
> Date: Wed, 20 Dec 2017 09:39:14 +0100
> Subject: [PATCH] drm/tegra: dc: Implement legacy blending
>
> This implements alpha blending on legacy display controllers (Tegra20,
> Tegra30 and Tegra114). While it's theoretically possible to support the
> zpos property to enable userspace to specify the Z-order of each plane
> individually, this is not currently supported and the same fixed Z-
> order as previously defined is used.

 Perhaps one variant of implementing zpos could be by making overlays 
 'virtual',
 so each virtual overlay will be backed by the real HW plane and we could 
 swap
 the HW planes of the virtual overlays, emulating zpos.

> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> the opaque formats are now supported.
>
> Reported-by: Dmitry Osipenko 
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/tegra/dc.c| 74 
> ++-
>  drivers/gpu/drm/tegra/dc.h| 13 
>  drivers/gpu/drm/tegra/fb.c| 12 ---
>  drivers/gpu/drm/tegra/plane.c | 41 
>  drivers/gpu/drm/tegra/plane.h |  3 ++
>  5 files changed, 116 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index bc65c314e00f..07c687d7f615 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int 
> in)
>   return dfixed_frac(inf);
>  }
>  
> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
> +static void
> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
> +   const struct tegra_dc_window *window)
>  {
> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
> +  BLEND_COLOR_KEY_NONE;
> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
> +  BLEND_COLOR_KEY_NONE;
> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
> +
> + /* enable alpha blending if window->alpha */
> + if (window->alpha) {
> + background |= BLEND_CONTROL_DEPENDENT;
> + foreground |= BLEND_CONTROL_ALPHA;
> + }

 I think dependent weight means that window doesn't have alpha 
 transparency. So
 we should set the dependent_weight mode for opaque formats and 
 alpha_weight for
 formats with alpha channel.
>>>
>>> According to the TRM, dependent weight means that its weight will be 1 -
>>> the sum of the other overlapping windows. And it certainly seems to work
>>> that way in my tests (see below).
>>
>> Yes, and you are hardcoding the blending modes regardless of the actual 

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Dmitry Osipenko
On 20.12.2017 23:16, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 21:01, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
 Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
 DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if
 requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with
 both modesetting and opentegra drivers. On older Tegra's each plane has
 a blending configuration which should be used to enable / disable alpha
 blending and right now the blending configs are hardcoded to disabled
 alpha blending. In order to support alpha formats properly, planes
 blending configuration must be adjusted, until then the alpha formats
 are equal to non-alpha.

 Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
 Signed-off-by: Dmitry Osipenko 
 ---
  drivers/gpu/drm/tegra/dc.c| 29 ++---
  drivers/gpu/drm/tegra/dc.h|  1 +
  drivers/gpu/drm/tegra/fb.c| 13 -
  drivers/gpu/drm/tegra/hub.c   |  3 ++-
  drivers/gpu/drm/tegra/plane.c | 22 +-
  drivers/gpu/drm/tegra/plane.h |  2 +-
  6 files changed, 39 insertions(+), 31 deletions(-)
>>>
>>> This kept bugging me, so I spent some time looking at the blending
>>> programming. I came up with the attached patch which seems to work
>>> for all scenarios and is fairly similar to your patch. It has the
>>> added benefit that we can keep support for more formats.
>>>
>>> Any comments?
>>>
>>> Thierry
>>> --- >8 ---
>>> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
>>> From: Thierry Reding 
>>> Date: Wed, 20 Dec 2017 09:39:14 +0100
>>> Subject: [PATCH] drm/tegra: dc: Implement legacy blending
>>>
>>> This implements alpha blending on legacy display controllers (Tegra20,
>>> Tegra30 and Tegra114). While it's theoretically possible to support the
>>> zpos property to enable userspace to specify the Z-order of each plane
>>> individually, this is not currently supported and the same fixed Z-
>>> order as previously defined is used.
>>
>> Perhaps one variant of implementing zpos could be by making overlays 
>> 'virtual',
>> so each virtual overlay will be backed by the real HW plane and we could swap
>> the HW planes of the virtual overlays, emulating zpos.
>>
>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>> the opaque formats are now supported.
>>>
>>> Reported-by: Dmitry Osipenko 
>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>> Signed-off-by: Thierry Reding 
>>> ---
>>>  drivers/gpu/drm/tegra/dc.c| 74 
>>> ++-
>>>  drivers/gpu/drm/tegra/dc.h| 13 
>>>  drivers/gpu/drm/tegra/fb.c| 12 ---
>>>  drivers/gpu/drm/tegra/plane.c | 41 
>>>  drivers/gpu/drm/tegra/plane.h |  3 ++
>>>  5 files changed, 116 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>> index bc65c314e00f..07c687d7f615 100644
>>> --- a/drivers/gpu/drm/tegra/dc.c
>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
>>> return dfixed_frac(inf);
>>>  }
>>>  
>>> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
>>> +static void
>>> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
>>> + const struct tegra_dc_window *window)
>>>  {
>>> +   u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
>>> +BLEND_COLOR_KEY_NONE;
>>> +   u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
>>> +BLEND_COLOR_KEY_NONE;
>>> +   u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
>>> +
>>> +   /* enable alpha blending if window->alpha */
>>> +   if (window->alpha) {
>>> +   background |= BLEND_CONTROL_DEPENDENT;
>>> +   foreground |= BLEND_CONTROL_ALPHA;
>>> +   }
>>
>> I think dependent weight means that window doesn't have alpha transparency. 
>> So
>> we should set the dependent_weight mode for opaque formats and alpha_weight 
>> for
>> formats with alpha channel.
> 
> According to the TRM, dependent weight means that its weight will be 1 -
> the sum of the other overlapping windows. And it certainly seems to work
> that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane
format. So even if underlying window has alpha format, it will be treated as it
is opaque.

>> If the above is correct, then I'm suggesting to not expose alpha formats, we
>> should properly test all combinations of blending of all the windows. In one
>> case you could apply my patch for now and 

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-21 Thread Dmitry Osipenko
On 20.12.2017 21:01, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
>> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if
>> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with
>> both modesetting and opentegra drivers. On older Tegra's each plane has
>> a blending configuration which should be used to enable / disable alpha
>> blending and right now the blending configs are hardcoded to disabled
>> alpha blending. In order to support alpha formats properly, planes
>> blending configuration must be adjusted, until then the alpha formats
>> are equal to non-alpha.
>>
>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/gpu/drm/tegra/dc.c| 29 ++---
>>  drivers/gpu/drm/tegra/dc.h|  1 +
>>  drivers/gpu/drm/tegra/fb.c| 13 -
>>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
>>  drivers/gpu/drm/tegra/plane.c | 22 +-
>>  drivers/gpu/drm/tegra/plane.h |  2 +-
>>  6 files changed, 39 insertions(+), 31 deletions(-)
> 
> This kept bugging me, so I spent some time looking at the blending
> programming. I came up with the attached patch which seems to work
> for all scenarios and is fairly similar to your patch. It has the
> added benefit that we can keep support for more formats.
> 
> Any comments?
> 
> Thierry
> --- >8 ---
> From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
> From: Thierry Reding 
> Date: Wed, 20 Dec 2017 09:39:14 +0100
> Subject: [PATCH] drm/tegra: dc: Implement legacy blending
> 
> This implements alpha blending on legacy display controllers (Tegra20,
> Tegra30 and Tegra114). While it's theoretically possible to support the
> zpos property to enable userspace to specify the Z-order of each plane
> individually, this is not currently supported and the same fixed Z-
> order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual',
so each virtual overlay will be backed by the real HW plane and we could swap
the HW planes of the virtual overlays, emulating zpos.

> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> the opaque formats are now supported.
> 
> Reported-by: Dmitry Osipenko 
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/tegra/dc.c| 74 
> ++-
>  drivers/gpu/drm/tegra/dc.h| 13 
>  drivers/gpu/drm/tegra/fb.c| 12 ---
>  drivers/gpu/drm/tegra/plane.c | 41 
>  drivers/gpu/drm/tegra/plane.h |  3 ++
>  5 files changed, 116 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index bc65c314e00f..07c687d7f615 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
>   return dfixed_frac(inf);
>  }
>  
> -static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
> +static void
> +tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
> +   const struct tegra_dc_window *window)
>  {
> + u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
> +  BLEND_COLOR_KEY_NONE;
> + u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
> +  BLEND_COLOR_KEY_NONE;
> + u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
> +
> + /* enable alpha blending if window->alpha */
> + if (window->alpha) {
> + background |= BLEND_CONTROL_DEPENDENT;
> + foreground |= BLEND_CONTROL_ALPHA;
> + }

I think dependent weight means that window doesn't have alpha transparency. So
we should set the dependent_weight mode for opaque formats and alpha_weight for
formats with alpha channel.

If the above is correct, then I'm suggesting to not expose alpha formats, we
should properly test all combinations of blending of all the windows. In one
case you could apply my patch for now and then me/you/we could work on a proper
legacy blending implementation based on your patch. In the other case I could
take your patch into v4 (cursor patch would have to be rabased in that case) and
we will correct blending sometime later. I don't mind either case, up to you to
decide.

Is there any ready-made testsuite for the DRM planes blending? Or you have made
a test by yourself? In the latter case, could you please share the code so that
I could test it too without burden of writing testcases from scratch?

> +
>   /*
>* Disable blending and assume Window A is the bottom-most window,
>* Window C is the top-most window 

Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30

2017-12-20 Thread Thierry Reding
On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke
> DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if
> requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with
> both modesetting and opentegra drivers. On older Tegra's each plane has
> a blending configuration which should be used to enable / disable alpha
> blending and right now the blending configs are hardcoded to disabled
> alpha blending. In order to support alpha formats properly, planes
> blending configuration must be adjusted, until then the alpha formats
> are equal to non-alpha.
> 
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/tegra/dc.c| 29 ++---
>  drivers/gpu/drm/tegra/dc.h|  1 +
>  drivers/gpu/drm/tegra/fb.c| 13 -
>  drivers/gpu/drm/tegra/hub.c   |  3 ++-
>  drivers/gpu/drm/tegra/plane.c | 22 +-
>  drivers/gpu/drm/tegra/plane.h |  2 +-
>  6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending
programming. I came up with the attached patch which seems to work
for all scenarios and is fairly similar to your patch. It has the
added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding 
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

This implements alpha blending on legacy display controllers (Tegra20,
Tegra30 and Tegra114). While it's theoretically possible to support the
zpos property to enable userspace to specify the Z-order of each plane
individually, this is not currently supported and the same fixed Z-
order as previously defined is used.

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
the opaque formats are now supported.

Reported-by: Dmitry Osipenko 
Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/dc.c| 74 ++-
 drivers/gpu/drm/tegra/dc.h| 13 
 drivers/gpu/drm/tegra/fb.c| 12 ---
 drivers/gpu/drm/tegra/plane.c | 41 
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index bc65c314e00f..07c687d7f615 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+ const struct tegra_dc_window *window)
 {
+   u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
+BLEND_COLOR_KEY_NONE;
+   u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+BLEND_COLOR_KEY_NONE;
+   u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+   /* enable alpha blending if window->alpha */
+   if (window->alpha) {
+   background |= BLEND_CONTROL_DEPENDENT;
+   foreground |= BLEND_CONTROL_ALPHA;
+   }
+
/*
 * 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_plane_writel(plane, 0x00, DC_WIN_BLEND_NOKEY);
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_1WIN);
+   tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
+   tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
 
switch (plane->index) {
case 0:
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_2WIN_X);
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_2WIN_Y);
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_3WIN_XY);
+   tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_X);
+   tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y);
+   tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY);
break;
 
case 1:
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_2WIN_X);
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_2WIN_Y);
-   tegra_plane_writel(plane, 0x00, DC_WIN_BLEND_3WIN_XY);
+   tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X);
+   tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y);
+   tegra_plane_writel(plane, background,