* [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
@ 2018-07-27 16:04 Michel Dänzer
[not found] ` <20180727160452.18212-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-07-27 16:04 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Michel Dänzer <michel.daenzer@amd.com>
We were only storing the FB provided by the client, but on CRTCs with
TearFree enabled, we use a separate FB. This could cause
drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
could result in a hang when waiting for the pending flip to complete. We
were trying to avoid that by always clearing drmmode_crtc->flip_pending
when TearFree is enabled, but that wasn't reliable, because
drmmode_crtc->tear_free can already be FALSE at this point when
disabling TearFree.
Now that we're keeping track of each CRTC's flip FB separately,
drmmode_flip_handler can reliably clear flip_pending, and we no longer
need the TearFree hack.
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
src/drmmode_display.c | 47 ++++++++++++++++++++++++-------------------
src/drmmode_display.h | 2 +-
2 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 92f58c157..e58e15d7b 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -3051,17 +3051,21 @@ drmmode_flip_abort(xf86CrtcPtr crtc, void *event_data)
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
drmmode_flipdata_ptr flipdata = event_data;
+ int crtc_id = drmmode_get_crtc_id(crtc);
+ struct drmmode_fb **fb = &flipdata->fb[crtc_id];
+
+ if (drmmode_crtc->flip_pending == *fb) {
+ drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
+ NULL);
+ }
+ drmmode_fb_reference(pAMDGPUEnt->fd, fb, NULL);
if (--flipdata->flip_count == 0) {
if (!flipdata->fe_crtc)
flipdata->fe_crtc = crtc;
flipdata->abort(flipdata->fe_crtc, flipdata->event_data);
- drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
free(flipdata);
}
-
- drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
- NULL);
}
static void
@@ -3070,6 +3074,8 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
drmmode_flipdata_ptr flipdata = event_data;
+ int crtc_id = drmmode_get_crtc_id(crtc);
+ struct drmmode_fb **fb = &flipdata->fb[crtc_id];
/* Is this the event whose info shall be delivered to higher level? */
if (crtc == flipdata->fe_crtc) {
@@ -3078,13 +3084,12 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
flipdata->fe_usec = usec;
}
- drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
- flipdata->fb);
- if (drmmode_crtc->tear_free ||
- drmmode_crtc->flip_pending == flipdata->fb) {
+ if (drmmode_crtc->flip_pending == *fb) {
drmmode_fb_reference(pAMDGPUEnt->fd,
&drmmode_crtc->flip_pending, NULL);
}
+ drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, *fb);
+ drmmode_fb_reference(pAMDGPUEnt->fd, fb, NULL);
if (--flipdata->flip_count == 0) {
/* Deliver MSC & UST from reference/current CRTC to flip event
@@ -3096,7 +3101,6 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
else
flipdata->handler(crtc, frame, usec, flipdata->event_data);
- drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
free(flipdata);
}
}
@@ -3875,21 +3879,22 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
xf86CrtcPtr crtc = NULL;
drmmode_crtc_private_ptr drmmode_crtc = config->crtc[0]->driver_private;
- int i;
uint32_t flip_flags = flip_sync == FLIP_ASYNC ? DRM_MODE_PAGE_FLIP_ASYNC : 0;
drmmode_flipdata_ptr flipdata;
uintptr_t drm_queue_seq = 0;
+ struct drmmode_fb *fb;
+ int i = 0;
- flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
+ flipdata = calloc(1, sizeof(*flipdata) + config->num_crtc *
+ sizeof(flipdata->fb[0]));
if (!flipdata) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"flip queue: data alloc failed.\n");
goto error;
}
- drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb,
- amdgpu_pixmap_get_fb(new_front));
- if (!flipdata->fb) {
+ fb = amdgpu_pixmap_get_fb(new_front);
+ if (!fb) {
ErrorF("Failed to get FB for flip\n");
goto error;
}
@@ -3910,8 +3915,6 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
flipdata->fe_crtc = ref_crtc;
for (i = 0; i < config->num_crtc; i++) {
- struct drmmode_fb *fb = flipdata->fb;
-
crtc = config->crtc[i];
drmmode_crtc = crtc->driver_private;
@@ -3947,8 +3950,9 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
goto next;
}
- fb = amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
- if (!fb) {
+ drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb[i],
+ amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
+ if (!flipdata->fb[i]) {
ErrorF("Failed to get FB for TearFree flip\n");
goto error;
}
@@ -3963,6 +3967,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending);
drmmode_crtc->scanout_update_pending = 0;
}
+ } else {
+ drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb[i], fb);
}
if (crtc == ref_crtc) {
@@ -3988,8 +3994,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
}
next:
- drmmode_fb_reference(pAMDGPUEnt->fd,
- &drmmode_crtc->flip_pending, fb);
+ drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
+ flipdata->fb[i]);
drm_queue_seq = 0;
}
@@ -4007,7 +4013,6 @@ error:
drmmode_flip_abort(crtc, flipdata);
else {
abort(NULL, data);
- drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
free(flipdata);
}
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 8b949f79d..5618c6b40 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -74,7 +74,6 @@ typedef struct {
} drmmode_rec, *drmmode_ptr;
typedef struct {
- struct drmmode_fb *fb;
void *event_data;
int flip_count;
unsigned int fe_frame;
@@ -82,6 +81,7 @@ typedef struct {
xf86CrtcPtr fe_crtc;
amdgpu_drm_handler_proc handler;
amdgpu_drm_abort_proc abort;
+ struct drmmode_fb *fb[0];
} drmmode_flipdata_rec, *drmmode_flipdata_ptr;
struct drmmode_fb {
--
2.18.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
[not found] ` <20180727160452.18212-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-07-30 18:12 ` Alex Deucher
[not found] ` <CADnq5_M+3OYaT1KdAZzqNv9uc-2ULQDots4CigjxMLLA2u3iUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-08-10 7:06 ` Johannes Hirte
1 sibling, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2018-07-30 18:12 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx list
On Fri, Jul 27, 2018 at 12:04 PM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> We were only storing the FB provided by the client, but on CRTCs with
> TearFree enabled, we use a separate FB. This could cause
> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
> could result in a hang when waiting for the pending flip to complete. We
> were trying to avoid that by always clearing drmmode_crtc->flip_pending
> when TearFree is enabled, but that wasn't reliable, because
> drmmode_crtc->tear_free can already be FALSE at this point when
> disabling TearFree.
>
> Now that we're keeping track of each CRTC's flip FB separately,
> drmmode_flip_handler can reliably clear flip_pending, and we no longer
> need the TearFree hack.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
> src/drmmode_display.c | 47 ++++++++++++++++++++++++-------------------
> src/drmmode_display.h | 2 +-
> 2 files changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 92f58c157..e58e15d7b 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -3051,17 +3051,21 @@ drmmode_flip_abort(xf86CrtcPtr crtc, void *event_data)
> drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> drmmode_flipdata_ptr flipdata = event_data;
> + int crtc_id = drmmode_get_crtc_id(crtc);
> + struct drmmode_fb **fb = &flipdata->fb[crtc_id];
> +
> + if (drmmode_crtc->flip_pending == *fb) {
> + drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
> + NULL);
> + }
> + drmmode_fb_reference(pAMDGPUEnt->fd, fb, NULL);
>
> if (--flipdata->flip_count == 0) {
> if (!flipdata->fe_crtc)
> flipdata->fe_crtc = crtc;
> flipdata->abort(flipdata->fe_crtc, flipdata->event_data);
> - drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
> free(flipdata);
> }
> -
> - drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
> - NULL);
> }
>
> static void
> @@ -3070,6 +3074,8 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
> AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
> drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> drmmode_flipdata_ptr flipdata = event_data;
> + int crtc_id = drmmode_get_crtc_id(crtc);
> + struct drmmode_fb **fb = &flipdata->fb[crtc_id];
>
> /* Is this the event whose info shall be delivered to higher level? */
> if (crtc == flipdata->fe_crtc) {
> @@ -3078,13 +3084,12 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
> flipdata->fe_usec = usec;
> }
>
> - drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
> - flipdata->fb);
> - if (drmmode_crtc->tear_free ||
> - drmmode_crtc->flip_pending == flipdata->fb) {
> + if (drmmode_crtc->flip_pending == *fb) {
> drmmode_fb_reference(pAMDGPUEnt->fd,
> &drmmode_crtc->flip_pending, NULL);
> }
> + drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, *fb);
> + drmmode_fb_reference(pAMDGPUEnt->fd, fb, NULL);
>
> if (--flipdata->flip_count == 0) {
> /* Deliver MSC & UST from reference/current CRTC to flip event
> @@ -3096,7 +3101,6 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
> else
> flipdata->handler(crtc, frame, usec, flipdata->event_data);
>
> - drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
> free(flipdata);
> }
> }
> @@ -3875,21 +3879,22 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
> xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
> xf86CrtcPtr crtc = NULL;
> drmmode_crtc_private_ptr drmmode_crtc = config->crtc[0]->driver_private;
> - int i;
> uint32_t flip_flags = flip_sync == FLIP_ASYNC ? DRM_MODE_PAGE_FLIP_ASYNC : 0;
> drmmode_flipdata_ptr flipdata;
> uintptr_t drm_queue_seq = 0;
> + struct drmmode_fb *fb;
> + int i = 0;
>
> - flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
> + flipdata = calloc(1, sizeof(*flipdata) + config->num_crtc *
> + sizeof(flipdata->fb[0]));
> if (!flipdata) {
> xf86DrvMsg(scrn->scrnIndex, X_WARNING,
> "flip queue: data alloc failed.\n");
> goto error;
> }
>
> - drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb,
> - amdgpu_pixmap_get_fb(new_front));
> - if (!flipdata->fb) {
> + fb = amdgpu_pixmap_get_fb(new_front);
> + if (!fb) {
> ErrorF("Failed to get FB for flip\n");
> goto error;
> }
> @@ -3910,8 +3915,6 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
> flipdata->fe_crtc = ref_crtc;
>
> for (i = 0; i < config->num_crtc; i++) {
> - struct drmmode_fb *fb = flipdata->fb;
> -
> crtc = config->crtc[i];
> drmmode_crtc = crtc->driver_private;
>
> @@ -3947,8 +3950,9 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
> goto next;
> }
>
> - fb = amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
> - if (!fb) {
> + drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb[i],
> + amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
> + if (!flipdata->fb[i]) {
> ErrorF("Failed to get FB for TearFree flip\n");
> goto error;
> }
> @@ -3963,6 +3967,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
> amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending);
> drmmode_crtc->scanout_update_pending = 0;
> }
> + } else {
> + drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb[i], fb);
> }
>
> if (crtc == ref_crtc) {
> @@ -3988,8 +3994,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
> }
>
> next:
> - drmmode_fb_reference(pAMDGPUEnt->fd,
> - &drmmode_crtc->flip_pending, fb);
> + drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
> + flipdata->fb[i]);
> drm_queue_seq = 0;
> }
>
> @@ -4007,7 +4013,6 @@ error:
> drmmode_flip_abort(crtc, flipdata);
> else {
> abort(NULL, data);
> - drmmode_fb_reference(pAMDGPUEnt->fd, &flipdata->fb, NULL);
> free(flipdata);
> }
>
> diff --git a/src/drmmode_display.h b/src/drmmode_display.h
> index 8b949f79d..5618c6b40 100644
> --- a/src/drmmode_display.h
> +++ b/src/drmmode_display.h
> @@ -74,7 +74,6 @@ typedef struct {
> } drmmode_rec, *drmmode_ptr;
>
> typedef struct {
> - struct drmmode_fb *fb;
> void *event_data;
> int flip_count;
> unsigned int fe_frame;
> @@ -82,6 +81,7 @@ typedef struct {
> xf86CrtcPtr fe_crtc;
> amdgpu_drm_handler_proc handler;
> amdgpu_drm_abort_proc abort;
> + struct drmmode_fb *fb[0];
Don't some compilers have problems with zero sized arrays? Other than
that looks good to me:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Alex
> } drmmode_flipdata_rec, *drmmode_flipdata_ptr;
>
> struct drmmode_fb {
> --
> 2.18.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
[not found] ` <CADnq5_M+3OYaT1KdAZzqNv9uc-2ULQDots4CigjxMLLA2u3iUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-07-31 8:48 ` Michel Dänzer
[not found] ` <39de6fce-5abc-cb0a-0ac9-2ac03d0fe415-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-07-31 8:48 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx list
On 2018-07-30 08:12 PM, Alex Deucher wrote:
> On Fri, Jul 27, 2018 at 12:04 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> We were only storing the FB provided by the client, but on CRTCs with
>> TearFree enabled, we use a separate FB. This could cause
>> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
>> could result in a hang when waiting for the pending flip to complete. We
>> were trying to avoid that by always clearing drmmode_crtc->flip_pending
>> when TearFree is enabled, but that wasn't reliable, because
>> drmmode_crtc->tear_free can already be FALSE at this point when
>> disabling TearFree.
>>
>> Now that we're keeping track of each CRTC's flip FB separately,
>> drmmode_flip_handler can reliably clear flip_pending, and we no longer
>> need the TearFree hack.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>
>> [...]
>>
>> @@ -82,6 +81,7 @@ typedef struct {
>> xf86CrtcPtr fe_crtc;
>> amdgpu_drm_handler_proc handler;
>> amdgpu_drm_abort_proc abort;
>> + struct drmmode_fb *fb[0];
>
> Don't some compilers have problems with zero sized arrays?
Are you thinking of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b7d245009e734588e553092f5c0b0bd788b3a55
? Per https://bugs.freedesktop.org/show_bug.cgi?id=66932#c24 , the
problem there was variable sized arrays being declared as [1], which
obviously can't work.
Declaring a [0] array at the end of a struct is a GCC extension, see
https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html#Zero-Length . clang
seems to handle this fine as well.
An alternative would be using the C99 syntax "[];" instead, but I think
the above is fine in this case.
> Other than that looks good to me:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Thanks!
--
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
[not found] ` <39de6fce-5abc-cb0a-0ac9-2ac03d0fe415-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-07-31 15:01 ` Alex Deucher
0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2018-07-31 15:01 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx list
On Tue, Jul 31, 2018 at 4:48 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 2018-07-30 08:12 PM, Alex Deucher wrote:
>> On Fri, Jul 27, 2018 at 12:04 PM, Michel Dänzer <michel@daenzer.net> wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> We were only storing the FB provided by the client, but on CRTCs with
>>> TearFree enabled, we use a separate FB. This could cause
>>> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
>>> could result in a hang when waiting for the pending flip to complete. We
>>> were trying to avoid that by always clearing drmmode_crtc->flip_pending
>>> when TearFree is enabled, but that wasn't reliable, because
>>> drmmode_crtc->tear_free can already be FALSE at this point when
>>> disabling TearFree.
>>>
>>> Now that we're keeping track of each CRTC's flip FB separately,
>>> drmmode_flip_handler can reliably clear flip_pending, and we no longer
>>> need the TearFree hack.
>>>
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> [...]
>>>
>>> @@ -82,6 +81,7 @@ typedef struct {
>>> xf86CrtcPtr fe_crtc;
>>> amdgpu_drm_handler_proc handler;
>>> amdgpu_drm_abort_proc abort;
>>> + struct drmmode_fb *fb[0];
>>
>> Don't some compilers have problems with zero sized arrays?
>
> Are you thinking of
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b7d245009e734588e553092f5c0b0bd788b3a55
> ? Per https://bugs.freedesktop.org/show_bug.cgi?id=66932#c24 , the
> problem there was variable sized arrays being declared as [1], which
> obviously can't work.
>
> Declaring a [0] array at the end of a struct is a GCC extension, see
> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html#Zero-Length . clang
> seems to handle this fine as well.
>
> An alternative would be using the C99 syntax "[];" instead, but I think
> the above is fine in this case.
Yup, that was the issue I was thinking of. No other concerns.
Alex
>
>
>> Other than that looks good to me:
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Thanks!
>
>
> --
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
[not found] ` <20180727160452.18212-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-07-30 18:12 ` Alex Deucher
@ 2018-08-10 7:06 ` Johannes Hirte
2018-08-16 14:38 ` Michel Dänzer
1 sibling, 1 reply; 7+ messages in thread
From: Johannes Hirte @ 2018-08-10 7:06 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2018 Jul 27, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> We were only storing the FB provided by the client, but on CRTCs with
> TearFree enabled, we use a separate FB. This could cause
> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
> could result in a hang when waiting for the pending flip to complete. We
> were trying to avoid that by always clearing drmmode_crtc->flip_pending
> when TearFree is enabled, but that wasn't reliable, because
> drmmode_crtc->tear_free can already be FALSE at this point when
> disabling TearFree.
>
> Now that we're keeping track of each CRTC's flip FB separately,
> drmmode_flip_handler can reliably clear flip_pending, and we no longer
> need the TearFree hack.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Since this change I get a black screen when login into KDE Plasma. I
have to switch to linux console and back for getting the X11 screen.
Additional the Xorg.log is spammed with:
[ 189.744] (WW) AMDGPU(0): get vblank counter failed: Invalid argument
[ 189.828] (WW) AMDGPU(0): flip queue failed in amdgpu_scanout_flip: Device or resource busy, TearFree inactive until next modeset
[ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
[ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
The "flip queue failed" message appears only once, the other two are
much more often.
System is a Carrizo A10-8700B, kernel 4.17.13 + this patch:
https://bugzilla.kernel.org/attachment.cgi?id=276173
--
Regards,
Johannes
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
2018-08-10 7:06 ` Johannes Hirte
@ 2018-08-16 14:38 ` Michel Dänzer
[not found] ` <1705f9ac-9824-a0f7-0556-6c3f9006bed6-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Michel Dänzer @ 2018-08-16 14:38 UTC (permalink / raw)
To: Johannes Hirte; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2018-08-10 09:06 AM, Johannes Hirte wrote:
> On 2018 Jul 27, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> We were only storing the FB provided by the client, but on CRTCs with
>> TearFree enabled, we use a separate FB. This could cause
>> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
>> could result in a hang when waiting for the pending flip to complete. We
>> were trying to avoid that by always clearing drmmode_crtc->flip_pending
>> when TearFree is enabled, but that wasn't reliable, because
>> drmmode_crtc->tear_free can already be FALSE at this point when
>> disabling TearFree.
>>
>> Now that we're keeping track of each CRTC's flip FB separately,
>> drmmode_flip_handler can reliably clear flip_pending, and we no longer
>> need the TearFree hack.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>
> Since this change I get a black screen when login into KDE Plasma. I
> have to switch to linux console and back for getting the X11 screen.
> Additional the Xorg.log is spammed with:
>
> [ 189.744] (WW) AMDGPU(0): get vblank counter failed: Invalid argument
> [ 189.828] (WW) AMDGPU(0): flip queue failed in amdgpu_scanout_flip: Device or resource busy, TearFree inactive until next modeset
> [ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
> [ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
>
> The "flip queue failed" message appears only once, the other two are
> much more often.
>
> System is a Carrizo A10-8700B, kernel 4.17.13 + this patch:
> https://bugzilla.kernel.org/attachment.cgi?id=276173
Does https://patchwork.freedesktop.org/patch/244860/ fix it?
--
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec
[not found] ` <1705f9ac-9824-a0f7-0556-6c3f9006bed6-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-08-17 12:22 ` Johannes Hirte
0 siblings, 0 replies; 7+ messages in thread
From: Johannes Hirte @ 2018-08-17 12:22 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2018 Aug 16, Michel Dänzer wrote:
> On 2018-08-10 09:06 AM, Johannes Hirte wrote:
> > On 2018 Jul 27, Michel Dänzer wrote:
> >> From: Michel Dänzer <michel.daenzer@amd.com>
> >>
> >> We were only storing the FB provided by the client, but on CRTCs with
> >> TearFree enabled, we use a separate FB. This could cause
> >> drmmode_flip_handler to fail to clear drmmode_crtc->flip_pending, which
> >> could result in a hang when waiting for the pending flip to complete. We
> >> were trying to avoid that by always clearing drmmode_crtc->flip_pending
> >> when TearFree is enabled, but that wasn't reliable, because
> >> drmmode_crtc->tear_free can already be FALSE at this point when
> >> disabling TearFree.
> >>
> >> Now that we're keeping track of each CRTC's flip FB separately,
> >> drmmode_flip_handler can reliably clear flip_pending, and we no longer
> >> need the TearFree hack.
> >>
> >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> >
> > Since this change I get a black screen when login into KDE Plasma. I
> > have to switch to linux console and back for getting the X11 screen.
> > Additional the Xorg.log is spammed with:
> >
> > [ 189.744] (WW) AMDGPU(0): get vblank counter failed: Invalid argument
> > [ 189.828] (WW) AMDGPU(0): flip queue failed in amdgpu_scanout_flip: Device or resource busy, TearFree inactive until next modeset
> > [ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
> > [ 189.828] (WW) AMDGPU(0): drmmode_wait_vblank failed for scanout update: Invalid argument
> >
> > The "flip queue failed" message appears only once, the other two are
> > much more often.
> >
> > System is a Carrizo A10-8700B, kernel 4.17.13 + this patch:
> > https://bugzilla.kernel.org/attachment.cgi?id=276173
>
> Does https://patchwork.freedesktop.org/patch/244860/ fix it?
>
Yes, this fixed it.
--
Regards,
Johannes
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-08-17 12:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 16:04 [PATCH xf86-video-amdgpu] Store FB for each CRTC in drmmode_flipdata_rec Michel Dänzer
[not found] ` <20180727160452.18212-1-michel-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-07-30 18:12 ` Alex Deucher
[not found] ` <CADnq5_M+3OYaT1KdAZzqNv9uc-2ULQDots4CigjxMLLA2u3iUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-07-31 8:48 ` Michel Dänzer
[not found] ` <39de6fce-5abc-cb0a-0ac9-2ac03d0fe415-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-07-31 15:01 ` Alex Deucher
2018-08-10 7:06 ` Johannes Hirte
2018-08-16 14:38 ` Michel Dänzer
[not found] ` <1705f9ac-9824-a0f7-0556-6c3f9006bed6-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-08-17 12:22 ` Johannes Hirte
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.