From: Daniel Vetter <daniel@ffwll.ch> To: "Michel Dänzer" <michel@daenzer.net> Cc: Leo Li <sunpeng.li@amd.com>, amd-gfx list <amd-gfx@lists.freedesktop.org>, Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>, dri-devel <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH] drm/amdgpu/dc: Simplify drm_crtc_state::active checks Date: Wed, 22 Jul 2020 14:51:31 +0200 [thread overview] Message-ID: <CAKMK7uGO3K0P1yJsdT0urEXOVyBCU_Gsm8JaAW3R0TrBkueYNg@mail.gmail.com> (raw) In-Reply-To: <20200722123813.721041-1-michel@daenzer.net> On Wed, Jul 22, 2020 at 2:38 PM Michel Dänzer <michel@daenzer.net> wrote: > > From: Michel Dänzer <mdaenzer@redhat.com> > > drm_atomic_crtc_check enforces that ::active can only be true if > ::enable is as well. > > Signed-off-by: Michel Dänzer <mdaenzer@redhat.com> modeset vs modereset is a bit an inglorious name choice ... since this seems to be glue code and not part of core dc, maybe rename to enable_required/disable_required to keep it consistent with the wording atomic helpers use? DC also seems to use reset for a lot of other things already (state reset, like atomic, or gpu reset like drm/scheduler's td_r_), so I think this would also help clarity from a DC perspective. Patch itself is good, above just an idea for another patch on top. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 312c543b258f..dabef307a74f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3415,21 +3415,12 @@ static bool modeset_required(struct drm_crtc_state *crtc_state, > struct dc_stream_state *new_stream, > struct dc_stream_state *old_stream) > { > - if (!drm_atomic_crtc_needs_modeset(crtc_state)) > - return false; > - > - if (!crtc_state->enable) > - return false; > - > - return crtc_state->active; > + return crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state); > } > > static bool modereset_required(struct drm_crtc_state *crtc_state) > { > - if (!drm_atomic_crtc_needs_modeset(crtc_state)) > - return false; > - > - return !crtc_state->enable || !crtc_state->active; > + return !crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state); > } > > static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder) > @@ -8108,8 +8099,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, > * We want to do dc stream updates that do not require a > * full modeset below. > */ > - if (!(enable && aconnector && new_crtc_state->enable && > - new_crtc_state->active)) > + if (!(enable && aconnector && new_crtc_state->active)) > return 0; > /* > * Given above conditions, the dc state cannot be NULL because: > -- > 2.28.0.rc0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ 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: Daniel Vetter <daniel@ffwll.ch> To: "Michel Dänzer" <michel@daenzer.net> Cc: Leo Li <sunpeng.li@amd.com>, amd-gfx list <amd-gfx@lists.freedesktop.org>, Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>, dri-devel <dri-devel@lists.freedesktop.org> Subject: Re: [PATCH] drm/amdgpu/dc: Simplify drm_crtc_state::active checks Date: Wed, 22 Jul 2020 14:51:31 +0200 [thread overview] Message-ID: <CAKMK7uGO3K0P1yJsdT0urEXOVyBCU_Gsm8JaAW3R0TrBkueYNg@mail.gmail.com> (raw) In-Reply-To: <20200722123813.721041-1-michel@daenzer.net> On Wed, Jul 22, 2020 at 2:38 PM Michel Dänzer <michel@daenzer.net> wrote: > > From: Michel Dänzer <mdaenzer@redhat.com> > > drm_atomic_crtc_check enforces that ::active can only be true if > ::enable is as well. > > Signed-off-by: Michel Dänzer <mdaenzer@redhat.com> modeset vs modereset is a bit an inglorious name choice ... since this seems to be glue code and not part of core dc, maybe rename to enable_required/disable_required to keep it consistent with the wording atomic helpers use? DC also seems to use reset for a lot of other things already (state reset, like atomic, or gpu reset like drm/scheduler's td_r_), so I think this would also help clarity from a DC perspective. Patch itself is good, above just an idea for another patch on top. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 312c543b258f..dabef307a74f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3415,21 +3415,12 @@ static bool modeset_required(struct drm_crtc_state *crtc_state, > struct dc_stream_state *new_stream, > struct dc_stream_state *old_stream) > { > - if (!drm_atomic_crtc_needs_modeset(crtc_state)) > - return false; > - > - if (!crtc_state->enable) > - return false; > - > - return crtc_state->active; > + return crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state); > } > > static bool modereset_required(struct drm_crtc_state *crtc_state) > { > - if (!drm_atomic_crtc_needs_modeset(crtc_state)) > - return false; > - > - return !crtc_state->enable || !crtc_state->active; > + return !crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state); > } > > static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder) > @@ -8108,8 +8099,7 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, > * We want to do dc stream updates that do not require a > * full modeset below. > */ > - if (!(enable && aconnector && new_crtc_state->enable && > - new_crtc_state->active)) > + if (!(enable && aconnector && new_crtc_state->active)) > return 0; > /* > * Given above conditions, the dc state cannot be NULL because: > -- > 2.28.0.rc0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2020-07-22 12:51 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-22 12:38 [PATCH] drm/amdgpu/dc: Simplify drm_crtc_state::active checks Michel Dänzer 2020-07-22 12:38 ` Michel Dänzer 2020-07-22 12:51 ` Daniel Vetter [this message] 2020-07-22 12:51 ` Daniel Vetter 2020-07-22 13:10 ` Kazlauskas, Nicholas 2020-07-22 13:10 ` Kazlauskas, Nicholas 2020-07-22 14:25 ` Michel Dänzer 2020-07-22 14:25 ` Michel Dänzer 2020-07-22 15:19 ` Daniel Vetter 2020-07-22 15:19 ` Daniel Vetter 2020-07-22 17:12 ` Alex Deucher 2020-07-22 17:12 ` Alex Deucher 2020-08-19 9:08 ` Michel Dänzer 2020-08-19 9:08 ` Michel Dänzer 2020-08-19 13:55 ` Alex Deucher 2020-08-19 13:55 ` 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=CAKMK7uGO3K0P1yJsdT0urEXOVyBCU_Gsm8JaAW3R0TrBkueYNg@mail.gmail.com \ --to=daniel@ffwll.ch \ --cc=amd-gfx@lists.freedesktop.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=michel@daenzer.net \ --cc=nicholas.kazlauskas@amd.com \ --cc=sunpeng.li@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: linkBe 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.