All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michel Dänzer" <michel@daenzer.net>
To: Daniel Vetter <daniel@ffwll.ch>,
	Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>,
	Harry Wentland <hwentlan@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
Date: Mon, 14 Sep 2020 09:52:35 +0200	[thread overview]
Message-ID: <d0c95272-9a1c-f9f0-f1b9-4e7ce1f319c7@daenzer.net> (raw)
In-Reply-To: <20200907075716.GO2352366@phenom.ffwll.local>

On 2020-09-07 9:57 a.m., Daniel Vetter wrote:
> On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote:
>> From: Michel Dänzer <mdaenzer@redhat.com>
>>
>> Don't check drm_crtc_state::active for this either, per its
>> documentation in include/drm/drm_crtc.h:
>>
>>   * Hence drivers must not consult @active in their various
>>   * &drm_mode_config_funcs.atomic_check callback to reject an atomic
>>   * commit.
>>
>> atomic_remove_fb disables the CRTC as needed for disabling the primary
>> plane.
>>
>> This prevents at least the following problems if the primary plane gets
>> disabled (e.g. due to destroying the FB assigned to the primary plane,
>> as happens e.g. with mutter in Wayland mode):
>>
>> * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
>>    (which enables the cursor plane).
>> * If the cursor plane was enabled, changing the legacy DPMS property
>>    value from off to on returned EINVAL.
>>
>> v2:
>> * Minor changes to code comment and commit log, per review feedback.
>>
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
> 
> Commit message agrees with my understand of this maze now, so:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks Daniel!


Nick / Harry, any more feedback? If not, can you apply this?


P.S. Since DCN doesn't make a distinction between primary or overlay 
planes in hardware, could ChromiumOS achieve the same effect with only 
the primary plane instead of only an overlay plane? If so, maybe there's 
no need for the "black plane hack".


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Michel Dänzer" <michel@daenzer.net>
To: Daniel Vetter <daniel@ffwll.ch>,
	Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>,
	Harry Wentland <hwentlan@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
Date: Mon, 14 Sep 2020 09:52:35 +0200	[thread overview]
Message-ID: <d0c95272-9a1c-f9f0-f1b9-4e7ce1f319c7@daenzer.net> (raw)
In-Reply-To: <20200907075716.GO2352366@phenom.ffwll.local>

On 2020-09-07 9:57 a.m., Daniel Vetter wrote:
> On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote:
>> From: Michel Dänzer <mdaenzer@redhat.com>
>>
>> Don't check drm_crtc_state::active for this either, per its
>> documentation in include/drm/drm_crtc.h:
>>
>>   * Hence drivers must not consult @active in their various
>>   * &drm_mode_config_funcs.atomic_check callback to reject an atomic
>>   * commit.
>>
>> atomic_remove_fb disables the CRTC as needed for disabling the primary
>> plane.
>>
>> This prevents at least the following problems if the primary plane gets
>> disabled (e.g. due to destroying the FB assigned to the primary plane,
>> as happens e.g. with mutter in Wayland mode):
>>
>> * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
>>    (which enables the cursor plane).
>> * If the cursor plane was enabled, changing the legacy DPMS property
>>    value from off to on returned EINVAL.
>>
>> v2:
>> * Minor changes to code comment and commit log, per review feedback.
>>
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165
>> GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
> 
> Commit message agrees with my understand of this maze now, so:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks Daniel!


Nick / Harry, any more feedback? If not, can you apply this?


P.S. Since DCN doesn't make a distinction between primary or overlay 
planes in hardware, could ChromiumOS achieve the same effect with only 
the primary plane instead of only an overlay plane? If so, maybe there's 
no need for the "black plane hack".


-- 
Earthling Michel Dänzer               |               https://redhat.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

  reply	other threads:[~2020-09-14  7:52 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-21 16:57 [PATCH] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is Michel Dänzer
2020-08-21 16:57 ` Michel Dänzer
2020-08-21 18:07 ` Kazlauskas, Nicholas
2020-08-21 18:07   ` Kazlauskas, Nicholas
2020-08-22  9:59   ` Michel Dänzer
2020-08-22  9:59     ` Michel Dänzer
2020-08-24  7:43     ` Pekka Paalanen
2020-08-24  7:43       ` Pekka Paalanen
2020-08-25 14:55       ` Michel Dänzer
2020-08-25 14:55         ` Michel Dänzer
2020-09-01  7:57         ` Daniel Vetter
2020-09-01  7:57           ` Daniel Vetter
2020-09-01  8:56           ` Michel Dänzer
2020-09-01  8:56             ` Michel Dänzer
2020-09-01 10:34             ` Daniel Vetter
2020-09-01 10:34               ` Daniel Vetter
2020-08-25 16:58     ` Kazlauskas, Nicholas
2020-08-25 16:58       ` Kazlauskas, Nicholas
2020-08-26  8:24       ` Pekka Paalanen
2020-08-26  8:24         ` Pekka Paalanen
2020-08-26  8:58         ` Michel Dänzer
2020-08-26  8:58           ` Michel Dänzer
2020-09-01  7:54         ` Daniel Vetter
2020-09-01  7:54           ` Daniel Vetter
2020-09-01 13:58           ` Harry Wentland
2020-09-01 13:58             ` Harry Wentland
2020-09-02  7:02             ` Daniel Vetter
2020-09-02  7:02               ` Daniel Vetter
2020-09-02  9:26               ` Michel Dänzer
2020-09-02  9:26                 ` Michel Dänzer
2020-09-04 10:43 ` [PATCH v2] " Michel Dänzer
2020-09-04 10:43   ` Michel Dänzer
2020-09-07  7:57   ` Daniel Vetter
2020-09-07  7:57     ` Daniel Vetter
2020-09-14  7:52     ` Michel Dänzer [this message]
2020-09-14  7:52       ` Michel Dänzer
2020-09-14 14:37       ` Kazlauskas, Nicholas
2020-09-14 14:37         ` Kazlauskas, Nicholas
2020-09-14 15:22         ` Michel Dänzer
2020-09-14 15:22           ` Michel Dänzer
2020-09-14 15:33           ` Kazlauskas, Nicholas
2020-09-14 15:33             ` Kazlauskas, Nicholas
2020-09-14 15:44             ` Michel Dänzer
2020-09-14 15:44               ` Michel Dänzer
2020-09-15 21:00         ` Alex Deucher
2020-09-15 21:00           ` Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d0c95272-9a1c-f9f0-f1b9-4e7ce1f319c7@daenzer.net \
    --to=michel@daenzer.net \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hwentlan@amd.com \
    --cc=nicholas.kazlauskas@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.