Re: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)"

[Harry Wentland]

On 2018-04-13 03:15 AM, Michel Dänzer wrote:
> On 2018-04-13 04:55 AM, S, Shirish wrote:
>> Hi Harry, Alex,
>>
>> The solution given while reviewing my patch was that "DC should support 
>> enabling a CRTC without a framebuffer."
> 
> I think it's weird that an enabled CRTC would end up having a NULL FB
> during normal operation in the first place. Any idea how that happens?
> 

I think we've only seen that with ChromeOS and in one case on Ubuntu. If I 
remember right it was when running GDM with Wayland and logging into X. I 
imagine our DDX driver never pulls this scenario on us.

Technically this would be a reasonable case for our HW, i.e. we could keep pipe 
running and blanked and just not scan-out a FB.

> 
>> Since the revert is a temporary workaround to address issue at hand and 
>> considering the bigger regression it will cause on ChromeOS(explained below),
>> I would strongly recommend that the revert should not be mainlined (to linus 
>> tree), until a proper fix for both the issues i.e., flickering and BUG hit 
>> on atomic is found.
>>
>> For the sake of everyone's understanding in the list below is a brief 
>> background.
>>
>> Mainline patch from intel folks, "846c7df drm/atomic: Try to preserve the 
>> crtc enabled state in drm_atomic_remove_fb, v2." 
>> introduces a slight behavioral change to rmfb. Instead of disabling a crtc 
>> when the primary plane is disabled, it now preserves it.
>>
>> This change leads to BUG hit while performing atomic commit on amd driver 
>> leading to reboot/system instability on ChromeOS which has enabled
>> drm atomic way of rendering, I also remember it causing some issue on other 
>> OS as well.
> 
> Can you provide more information about "some issue on other OS"?
> Otherwise I'm afraid this is a no-brainer, since nobody's using ChromeOS
> with an AMD GPU in the wild, whereas many people have been running into
> issues with these commits in the wild, especially since they're in the
> 4.16 release.
> 

I tend to agree with Michel here, there's been quite the fallout from that 
patch for people's daily drivers.

Harry

> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)"

[Michel Dänzer]

On 2018-04-13 04:55 AM, S, Shirish wrote:
> Hi Harry, Alex,
> 
> The solution given while reviewing my patch was that "DC should support 
> enabling a CRTC without a framebuffer."

I think it's weird that an enabled CRTC would end up having a NULL FB
during normal operation in the first place. Any idea how that happens?


> Since the revert is a temporary workaround to address issue at hand and 
> considering the bigger regression it will cause on ChromeOS(explained below),
> I would strongly recommend that the revert should not be mainlined (to linus 
> tree), until a proper fix for both the issues i.e., flickering and BUG hit on 
> atomic is found.
> 
> For the sake of everyone's understanding in the list below is a brief 
> background.
> 
> Mainline patch from intel folks, "846c7df drm/atomic: Try to preserve the 
> crtc enabled state in drm_atomic_remove_fb, v2." 
> introduces a slight behavioral change to rmfb. Instead of disabling a crtc 
> when the primary plane is disabled, it now preserves it.
> 
> This change leads to BUG hit while performing atomic commit on amd driver 
> leading to reboot/system instability on ChromeOS which has enabled
> drm atomic way of rendering, I also remember it causing some issue on other 
> OS as well.

Can you provide more information about "some issue on other OS"?
Otherwise I'm afraid this is a no-brainer, since nobody's using ChromeOS
with an AMD GPU in the wild, whereas many people have been running into
issues with these commits in the wild, especially since they're in the
4.16 release.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)"

[S, Shirish]

Hi Harry, Alex,

The solution given while reviewing my patch was that "DC should support 
enabling a CRTC without a framebuffer."

Since the revert is a temporary workaround to address issue at hand and 
considering the bigger regression it will cause on ChromeOS(explained below),
I would strongly recommend that the revert should not be mainlined (to linus 
tree), until a proper fix for both the issues i.e., flickering and BUG hit on 
atomic is found.

For the sake of everyone's understanding in the list below is a brief 
background.

Mainline patch from intel folks, "846c7df drm/atomic: Try to preserve the crtc 
enabled state in drm_atomic_remove_fb, v2." 
introduces a slight behavioral change to rmfb. Instead of disabling a crtc when 
the primary plane is disabled, it now preserves it.

This change leads to BUG hit while performing atomic commit on amd driver 
leading to reboot/system instability on ChromeOS which has enabled
drm atomic way of rendering, I also remember it causing some issue on other OS 
as well.

Thanks & Regards,
Shirish S

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net] 
Sent: Thursday, April 12, 2018 8:39 PM
To: Wentland, Harry <harry.wentl...@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
<alexander.deuc...@amd.com>; S, Shirish <shiris...@amd.com>
Subject: Re: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on 
their primary plane (V2)"

On 2018-04-12 04:51 PM, Harry Wentland wrote:
> This seems to cause flickering and lock-ups for a wide range of users.
> Revert until we've found a proper fix for the flickering and lock-ups.
> 
> This reverts commit 36cc549d59864b7161f0e23d710c1c4d1b9cf022.
> 
> Cc: Shirish S <shiris...@amd.com>
> Cc: Alex Deucher <alexander.deuc...@amd.com>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Harry Wentland <harry.wentl...@amd.com>

Thanks Harry, both patches are

Reviewed-by: Michel Dänzer <michel.daen...@amd.com>


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] Revert "drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)"

[Michel Dänzer]

On 2018-04-12 04:51 PM, Harry Wentland wrote:
> This seems to cause flickering and lock-ups for a wide range of users.
> Revert until we've found a proper fix for the flickering and lock-ups.
> 
> This reverts commit 36cc549d59864b7161f0e23d710c1c4d1b9cf022.
> 
> Cc: Shirish S 
> Cc: Alex Deucher 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Harry Wentland 

Thanks Harry, both patches are

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx