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 <[email protected]>
>>>>>>> ---
>>>>>>> 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 <[email protected]>
>>>>>> 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 <[email protected]>
>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>>> Signed-off-by: Thierry Reding <[email protected]>
>>>>>> ---
>>>>>> 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.
>>
>> Ah yes, indeed. The patch currently assumes that if the current plane
>> has an alpha component, then all the others will, too. That can probably
>> be done fairly easily by looking at the current atomic state and
>> inspecting the pixel format for each plane on the same CRTC. Let me take
>> a stab at that tomorrow.
>
> Okay, please push patch to the public repo once it is ready and let me know.
> I'll rebase cursor patch on it.
>
>> I'm wondering if we can deal with zpos, too, if we're already going
>> through the trouble of looking at all planes involved. I think we can
>> simply permute the WIN_BLEND register writes depending on the Z-order
>> to implement proper zpos support.
> I think we will have to permute the whole window state, not only the WIN_BLEND
> register.
>
> Also I'm not sure how to make topmost window opaque and underlying windows
> transparent, will have to check how blending works. Maybe it not possible at
> all..
Although it is simple to implement using the fixed weights for 2WIN cases, but
then we will have to enhance the legacy blending code a tad to handle this case
properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can
still apply my patch that drops alpha formats on T20/30 and we will try to
implement alpha + zpos for 4.17.
