* [PATCH] gpu/drm: make crtc check before new_connector circle
@ 2020-11-02 2:58 Bernard Zhao
2020-11-02 10:17 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Bernard Zhao @ 2020-11-02 2:58 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel, linux-kernel
Cc: opensource.kernel, Bernard Zhao
In function prepare_signaling, crtc check (c==0) is not related
with the next new_connector circle, maybe we can put the crtc
check just after the crtc circle and before new_connector circle.
This change is to make the code to run a bit first.
Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
drivers/gpu/drm/drm_atomic_uapi.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 25c269bc4681..566110996474 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1182,6 +1182,12 @@ static int prepare_signaling(struct drm_device *dev,
c++;
}
+ /*
+ * Having this flag means user mode pends on event which will never
+ * reach due to lack of at least one CRTC for signaling
+ */
+ if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
+ return -EINVAL;
for_each_new_connector_in_state(state, conn, conn_state, i) {
struct drm_writeback_connector *wb_conn;
@@ -1220,13 +1226,6 @@ static int prepare_signaling(struct drm_device *dev,
conn_state->writeback_job->out_fence = fence;
}
- /*
- * Having this flag means user mode pends on event which will never
- * reach due to lack of at least one CRTC for signaling
- */
- if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
- return -EINVAL;
-
return 0;
}
--
2.29.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpu/drm: make crtc check before new_connector circle
2020-11-02 2:58 [PATCH] gpu/drm: make crtc check before new_connector circle Bernard Zhao
@ 2020-11-02 10:17 ` Daniel Vetter
2020-11-02 13:13 ` Bernard
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2020-11-02 10:17 UTC (permalink / raw)
To: Bernard Zhao
Cc: opensource.kernel, Thomas Zimmermann, David Airlie, linux-kernel,
dri-devel
On Sun, Nov 01, 2020 at 06:58:51PM -0800, Bernard Zhao wrote:
> In function prepare_signaling, crtc check (c==0) is not related
> with the next new_connector circle, maybe we can put the crtc
> check just after the crtc circle and before new_connector circle.
> This change is to make the code to run a bit first.
I'm a bit confused here with your explanation, I'm not understanding why
you do this change ... Can you pls elaborate? Maybe give an example or
something of the problem this patch solves, that often helps.
Thanks, Daniel
>
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 25c269bc4681..566110996474 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1182,6 +1182,12 @@ static int prepare_signaling(struct drm_device *dev,
>
> c++;
> }
> + /*
> + * Having this flag means user mode pends on event which will never
> + * reach due to lack of at least one CRTC for signaling
> + */
> + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> + return -EINVAL;
>
> for_each_new_connector_in_state(state, conn, conn_state, i) {
> struct drm_writeback_connector *wb_conn;
> @@ -1220,13 +1226,6 @@ static int prepare_signaling(struct drm_device *dev,
> conn_state->writeback_job->out_fence = fence;
> }
>
> - /*
> - * Having this flag means user mode pends on event which will never
> - * reach due to lack of at least one CRTC for signaling
> - */
> - if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> - return -EINVAL;
> -
> return 0;
> }
>
> --
> 2.29.0
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re:Re: [PATCH] gpu/drm: make crtc check before new_connector circle
2020-11-02 10:17 ` Daniel Vetter
@ 2020-11-02 13:13 ` Bernard
2020-11-02 13:17 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Bernard @ 2020-11-02 13:13 UTC (permalink / raw)
To: Daniel Vetter
Cc: opensource.kernel, Thomas Zimmermann, David Airlie, linux-kernel,
dri-devel
From: Daniel Vetter <daniel@ffwll.ch>
Date: 2020-11-02 18:17:24
To: Bernard Zhao <bernard@vivo.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,Maxime Ripard <mripard@kernel.org>,Thomas Zimmermann <tzimmermann@suse.de>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org,opensource.kernel@vivo.com
Subject: Re: [PATCH] gpu/drm: make crtc check before new_connector circle>On Sun, Nov 01, 2020 at 06:58:51PM -0800, Bernard Zhao wrote:
>> In function prepare_signaling, crtc check (c==0) is not related
>> with the next new_connector circle, maybe we can put the crtc
>> check just after the crtc circle and before new_connector circle.
>> This change is to make the code to run a bit first.
>
>I'm a bit confused here with your explanation, I'm not understanding why
>you do this change ... Can you pls elaborate? Maybe give an example or
>something of the problem this patch solves, that often helps.
>
>Thanks, Daniel
Hi:
This change is to make the function return error earlier when run into the error branch:
if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
return -EINVAL;
There two main FOR circles in this function, and the second FOR circle never changes the if condition("(c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))") variable`s value, like c & arg->flags.
So I think maybe we can check this condition before the second for circle, and return the error earlier when run into this error branch.
BR//Bernard
>>
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>> drivers/gpu/drm/drm_atomic_uapi.c | 13 ++++++-------
>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 25c269bc4681..566110996474 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -1182,6 +1182,12 @@ static int prepare_signaling(struct drm_device *dev,
>>
>> c++;
>> }
>> + /*
>> + * Having this flag means user mode pends on event which will never
>> + * reach due to lack of at least one CRTC for signaling
>> + */
>> + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
>> + return -EINVAL;
>>
>> for_each_new_connector_in_state(state, conn, conn_state, i) {
>> struct drm_writeback_connector *wb_conn;
>> @@ -1220,13 +1226,6 @@ static int prepare_signaling(struct drm_device *dev,
>> conn_state->writeback_job->out_fence = fence;
>> }
>>
>> - /*
>> - * Having this flag means user mode pends on event which will never
>> - * reach due to lack of at least one CRTC for signaling
>> - */
>> - if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
>> - return -EINVAL;
>> -
>> return 0;
>> }
>>
>> --
>> 2.29.0
>>
>
>--
>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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] gpu/drm: make crtc check before new_connector circle
2020-11-02 13:13 ` Bernard
@ 2020-11-02 13:17 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2020-11-02 13:17 UTC (permalink / raw)
To: Bernard
Cc: opensource.kernel, David Airlie, linux-kernel, dri-devel,
Thomas Zimmermann
On Mon, Nov 02, 2020 at 09:13:30PM +0800, Bernard wrote:
>
>
> From: Daniel Vetter <daniel@ffwll.ch>
> Date: 2020-11-02 18:17:24
> To: Bernard Zhao <bernard@vivo.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,Maxime Ripard <mripard@kernel.org>,Thomas Zimmermann <tzimmermann@suse.de>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org,opensource.kernel@vivo.com
> Subject: Re: [PATCH] gpu/drm: make crtc check before new_connector circle>On Sun, Nov 01, 2020 at 06:58:51PM -0800, Bernard Zhao wrote:
> >> In function prepare_signaling, crtc check (c==0) is not related
> >> with the next new_connector circle, maybe we can put the crtc
> >> check just after the crtc circle and before new_connector circle.
> >> This change is to make the code to run a bit first.
> >
> >I'm a bit confused here with your explanation, I'm not understanding why
> >you do this change ... Can you pls elaborate? Maybe give an example or
> >something of the problem this patch solves, that often helps.
> >
> >Thanks, Daniel
>
> Hi:
>
> This change is to make the function return error earlier when run into the error branch:
> if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> return -EINVAL;
> There two main FOR circles in this function, and the second FOR circle
> never changes the if condition("(c == 0 && (arg->flags &
> DRM_MODE_PAGE_FLIP_EVENT))") variable`s value, like c & arg->flags. So
> I think maybe we can check this condition before the second for circle,
> and return the error earlier when run into this error branch.
Ah ok. Makes sense, but this case is only ever hit for bad userspace that
got something wrong, so I'm not sure we should optimize for this. And with
this we kinda bury this fairly important check in the middle, so I don't
think it improves code readability.
-Daniel
>
> BR//Bernard
>
> >>
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >> drivers/gpu/drm/drm_atomic_uapi.c | 13 ++++++-------
> >> 1 file changed, 6 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 25c269bc4681..566110996474 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -1182,6 +1182,12 @@ static int prepare_signaling(struct drm_device *dev,
> >>
> >> c++;
> >> }
> >> + /*
> >> + * Having this flag means user mode pends on event which will never
> >> + * reach due to lack of at least one CRTC for signaling
> >> + */
> >> + if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> >> + return -EINVAL;
> >>
> >> for_each_new_connector_in_state(state, conn, conn_state, i) {
> >> struct drm_writeback_connector *wb_conn;
> >> @@ -1220,13 +1226,6 @@ static int prepare_signaling(struct drm_device *dev,
> >> conn_state->writeback_job->out_fence = fence;
> >> }
> >>
> >> - /*
> >> - * Having this flag means user mode pends on event which will never
> >> - * reach due to lack of at least one CRTC for signaling
> >> - */
> >> - if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> >> - return -EINVAL;
> >> -
> >> return 0;
> >> }
> >>
> >> --
> >> 2.29.0
> >>
> >
> >--
> >Daniel Vetter
> >Software Engineer, Intel Corporation
> >http://blog.ffwll.ch
>
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-02 13:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 2:58 [PATCH] gpu/drm: make crtc check before new_connector circle Bernard Zhao
2020-11-02 10:17 ` Daniel Vetter
2020-11-02 13:13 ` Bernard
2020-11-02 13:17 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).