* Re: [PATCH v3] Return only active connectors for get_resources ioctl
2018-11-29 11:09 [PATCH v3] Return only active connectors for get_resources ioctl Stanislav Lisovskiy
@ 2018-11-29 11:21 ` Greg KH
2018-11-29 11:30 ` ✓ Fi.CI.BAT: success for Return only active connectors for get_resources ioctl (rev4) Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2018-11-29 11:21 UTC (permalink / raw)
To: Stanislav Lisovskiy
Cc: dri-devel, martin.peres, ville.syrjala, chris, stable, intel-gfx,
daniel, lyude
On Thu, Nov 29, 2018 at 01:09:21PM +0200, Stanislav Lisovskiy wrote:
> Currently kernel might allocate different connector ids
> for the same outputs in case of DP MST, which seems to
> confuse userspace. There are can be different connector
> ids in the list, which could be assigned to the same
> output, while being in different states.
> This results in issues, like external displays staying
> blank after quick unplugging and plugging back(bug #106250).
> Returning only active DP connectors fixes the issue.
>
> v2: Removed caps from the title
>
> v3: Refactored initial condition to look more compact.
> Thanks to Chris Wilson for giving a hint.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply [flat|nested] 8+ messages in thread* ✓ Fi.CI.BAT: success for Return only active connectors for get_resources ioctl (rev4)
2018-11-29 11:09 [PATCH v3] Return only active connectors for get_resources ioctl Stanislav Lisovskiy
2018-11-29 11:21 ` Greg KH
@ 2018-11-29 11:30 ` Patchwork
2018-11-29 19:06 ` Lyude Paul
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-11-29 11:30 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: Return only active connectors for get_resources ioctl (rev4)
URL : https://patchwork.freedesktop.org/series/53163/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5221 -> Patchwork_10949
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/53163/revisions/4/mbox/
Known issues
------------
Here are the changes found in Patchwork_10949 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]
* igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] +1
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s4-devices:
- fi-ivb-3520m: FAIL [fdo#108880] -> PASS
* igt@gem_mmap@basic-small-bo:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
#### Warnings ####
* igt@i915_selftest@live_contexts:
- {fi-icl-u3}: INCOMPLETE [fdo#108315] -> DMESG-FAIL [fdo#108569]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (45 -> 41)
------------------------------
Additional (1): fi-gdg-551
Missing (5): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600
Build changes
-------------
* Linux: CI_DRM_5221 -> Patchwork_10949
CI_DRM_5221: 4c3e98f6787fab0ce3c5d20d25ebd538dfe234af @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4735: b05c028ccdb6ac8e8d8499a041bb14dfe358ee26 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10949: fbec8c5459ca5b586ceb8a3997f9b7df0ea59baa @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
fbec8c5459ca Return only active connectors for get_resources ioctl
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10949/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] Return only active connectors for get_resources ioctl
2018-11-29 11:09 [PATCH v3] Return only active connectors for get_resources ioctl Stanislav Lisovskiy
@ 2018-11-29 19:06 ` Lyude Paul
2018-11-29 11:30 ` ✓ Fi.CI.BAT: success for Return only active connectors for get_resources ioctl (rev4) Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2018-11-29 19:06 UTC (permalink / raw)
To: Stanislav Lisovskiy, dri-devel
Cc: martin.peres, ville.syrjala, chris, stable, intel-gfx, daniel
On Thu, 2018-11-29 at 13:09 +0200, Stanislav Lisovskiy wrote:
> Currently kernel might allocate different connector ids
> for the same outputs in case of DP MST, which seems to
> confuse userspace. There are can be different connector
> ids in the list, which could be assigned to the same
> output, while being in different states.
> This results in issues, like external displays staying
> blank after quick unplugging and plugging back(bug #106250).
> Returning only active DP connectors fixes the issue.
>
> v2: Removed caps from the title
>
> v3: Refactored initial condition to look more compact.
> Thanks to Chris Wilson for giving a hint.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..3e2cd959a66a 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -143,22 +143,28 @@ int drm_mode_getresources(struct drm_device *dev, void
> *data,
> drm_connector_list_iter_begin(dev, &conn_iter);
> count = 0;
> connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
> + DRM_DEBUG_KMS("GetResources: writing connectors start");
> drm_for_each_connector_iter(connector, &conn_iter) {
> /* only expose writeback connectors if userspace understands
> them */
> if (!file_priv->writeback_connectors &&
> (connector->connector_type ==
> DRM_MODE_CONNECTOR_WRITEBACK))
> continue;
>
> + if (READ_ONCE(connector->registration_state) !=
> DRM_CONNECTOR_REGISTERED)
> + continue;
> +
This conditional should just use drm_connector_is_unregistered() instead I'm
fairly sure.
> if (drm_lease_held(file_priv, connector->base.id)) {
> if (count < card_res->count_connectors &&
> put_user(connector->base.id, connector_id +
> count)) {
> drm_connector_list_iter_end(&conn_iter);
> return -EFAULT;
> }
> + DRM_DEBUG_KMS("GetResources: connector %s", connector-
> >name);
> count++;
> }
> }
> card_res->count_connectors = count;
> + DRM_DEBUG_KMS("GetResources: writing connectors end - count %d",
> count);
> drm_connector_list_iter_end(&conn_iter);
>
> return ret;
--
Cheers,
Lyude Paul
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] Return only active connectors for get_resources ioctl
@ 2018-11-29 19:06 ` Lyude Paul
0 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2018-11-29 19:06 UTC (permalink / raw)
To: Stanislav Lisovskiy, dri-devel
Cc: ville.syrjala, intel-gfx, martin.peres, stable
On Thu, 2018-11-29 at 13:09 +0200, Stanislav Lisovskiy wrote:
> Currently kernel might allocate different connector ids
> for the same outputs in case of DP MST, which seems to
> confuse userspace. There are can be different connector
> ids in the list, which could be assigned to the same
> output, while being in different states.
> This results in issues, like external displays staying
> blank after quick unplugging and plugging back(bug #106250).
> Returning only active DP connectors fixes the issue.
>
> v2: Removed caps from the title
>
> v3: Refactored initial condition to look more compact.
> Thanks to Chris Wilson for giving a hint.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..3e2cd959a66a 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -143,22 +143,28 @@ int drm_mode_getresources(struct drm_device *dev, void
> *data,
> drm_connector_list_iter_begin(dev, &conn_iter);
> count = 0;
> connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
> + DRM_DEBUG_KMS("GetResources: writing connectors start");
> drm_for_each_connector_iter(connector, &conn_iter) {
> /* only expose writeback connectors if userspace understands
> them */
> if (!file_priv->writeback_connectors &&
> (connector->connector_type ==
> DRM_MODE_CONNECTOR_WRITEBACK))
> continue;
>
> + if (READ_ONCE(connector->registration_state) !=
> DRM_CONNECTOR_REGISTERED)
> + continue;
> +
This conditional should just use drm_connector_is_unregistered() instead I'm
fairly sure.
> if (drm_lease_held(file_priv, connector->base.id)) {
> if (count < card_res->count_connectors &&
> put_user(connector->base.id, connector_id +
> count)) {
> drm_connector_list_iter_end(&conn_iter);
> return -EFAULT;
> }
> + DRM_DEBUG_KMS("GetResources: connector %s", connector-
> >name);
> count++;
> }
> }
> card_res->count_connectors = count;
> + DRM_DEBUG_KMS("GetResources: writing connectors end - count %d",
> count);
> drm_connector_list_iter_end(&conn_iter);
>
> return ret;
--
Cheers,
Lyude Paul
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.IGT: success for Return only active connectors for get_resources ioctl (rev4)
2018-11-29 11:09 [PATCH v3] Return only active connectors for get_resources ioctl Stanislav Lisovskiy
` (2 preceding siblings ...)
2018-11-29 19:06 ` Lyude Paul
@ 2018-11-29 21:00 ` Patchwork
2018-12-04 12:45 ` Jani Nikula
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-11-29 21:00 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: Return only active connectors for get_resources ioctl (rev4)
URL : https://patchwork.freedesktop.org/series/53163/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5221_full -> Patchwork_10949_full
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_10949_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10949_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_10949_full:
### IGT changes ###
#### Warnings ####
* igt@perf_pmu@rc6:
- shard-kbl: PASS -> SKIP
* igt@tools_test@tools_test:
- shard-skl: PASS -> SKIP
Known issues
------------
Here are the changes found in Patchwork_10949_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@drm_import_export@import-close-race-flink:
- shard-skl: PASS -> TIMEOUT [fdo#108667]
* igt@gem_ppgtt@blt-vs-render-ctxn:
- shard-skl: NOTRUN -> INCOMPLETE [fdo#106887]
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107956] +3
* igt@kms_cursor_crc@cursor-128x128-random:
- shard-apl: PASS -> FAIL [fdo#103232] +3
* igt@kms_cursor_crc@cursor-256x256-offscreen:
- shard-skl: PASS -> FAIL [fdo#103232]
* igt@kms_cursor_crc@cursor-256x256-suspend:
- shard-kbl: PASS -> DMESG-WARN [fdo#103313]
* igt@kms_cursor_crc@cursor-256x85-random:
- shard-glk: PASS -> FAIL [fdo#103232] +1
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
- shard-glk: PASS -> FAIL [fdo#104873]
* igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
- {shard-iclb}: PASS -> WARN [fdo#108336] +5
* igt@kms_fbcon_fbt@psr-suspend:
- shard-skl: NOTRUN -> FAIL [fdo#107882] +1
* igt@kms_flip@busy-flip:
- shard-skl: PASS -> FAIL [fdo#103257]
* igt@kms_flip@flip-vs-expired-vblank:
- shard-skl: PASS -> FAIL [fdo#105363]
* igt@kms_flip_tiling@flip-to-y-tiled:
- shard-skl: PASS -> FAIL [fdo#107931]
* igt@kms_frontbuffer_tracking@fbc-1p-rte:
- shard-apl: PASS -> FAIL [fdo#103167] / [fdo#105682]
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
- shard-glk: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- {shard-iclb}: PASS -> DMESG-FAIL [fdo#107724] +4
* igt@kms_frontbuffer_tracking@fbcpsr-indfb-scaledprimary:
- shard-skl: PASS -> FAIL [fdo#105682]
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +18
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
- {shard-iclb}: PASS -> FAIL [fdo#103167] +6
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]
* igt@kms_plane@plane-position-covered-pipe-a-planes:
- {shard-iclb}: PASS -> FAIL [fdo#103166] +2
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-skl: NOTRUN -> FAIL [fdo#108145] +2
* igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
- shard-skl: PASS -> FAIL [fdo#107815] / [fdo#108145]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-apl: PASS -> FAIL [fdo#103166]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
- shard-glk: PASS -> FAIL [fdo#103166]
* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-skl: NOTRUN -> FAIL [fdo#103166] / [fdo#107815]
* igt@kms_properties@connector-properties-atomic:
- shard-skl: NOTRUN -> FAIL [fdo#108642]
* igt@kms_setmode@basic:
- shard-kbl: PASS -> FAIL [fdo#99912]
* igt@kms_vblank@pipe-c-query-busy:
- shard-apl: PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +18
* igt@kms_vblank@pipe-c-wait-forked:
- {shard-iclb}: PASS -> DMESG-WARN [fdo#107724] +32
* igt@pm_rpm@gem-evict-pwrite:
- shard-skl: PASS -> INCOMPLETE [fdo#107807]
* igt@pm_rpm@i2c:
- {shard-iclb}: PASS -> FAIL [fdo#104097]
#### Possible fixes ####
* igt@gem_softpin@noreloc-s3:
- shard-skl: INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- {shard-iclb}: DMESG-WARN [fdo#107956] -> PASS
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
- shard-skl: DMESG-WARN [fdo#107956] -> PASS
* igt@kms_chv_cursor_fail@pipe-a-128x128-bottom-edge:
- {shard-iclb}: DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +2
* igt@kms_color@pipe-a-ctm-max:
- shard-apl: FAIL [fdo#108147] -> PASS
* igt@kms_cursor_crc@cursor-128x128-suspend:
- shard-skl: INCOMPLETE [fdo#104108] -> PASS +1
* igt@kms_cursor_crc@cursor-64x21-random:
- shard-apl: DMESG-FAIL [fdo#103232] / [fdo#103558] / [fdo#105602] -> PASS +1
* igt@kms_cursor_crc@cursor-64x21-sliding:
- shard-apl: FAIL [fdo#103232] -> PASS +1
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-glk: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS
- shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS
* igt@kms_flip@2x-flip-vs-dpms:
- shard-hsw: DMESG-WARN [fdo#102614] -> PASS
* igt@kms_flip@flip-vs-expired-vblank:
- shard-apl: FAIL [fdo#102887] / [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
- shard-apl: FAIL [fdo#103167] -> PASS +3
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
- {shard-iclb}: FAIL [fdo#103167] -> PASS +3
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
- shard-glk: FAIL [fdo#103167] -> PASS +2
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt:
- {shard-iclb}: DMESG-FAIL [fdo#107724] -> PASS
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
- shard-apl: DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +24
- shard-skl: FAIL [fdo#103191] / [fdo#107362] -> PASS
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- {shard-iclb}: DMESG-FAIL [fdo#103166] / [fdo#107724] -> PASS
* igt@kms_plane@plane-position-covered-pipe-a-planes:
- shard-apl: FAIL [fdo#103166] -> PASS +1
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- shard-glk: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: FAIL [fdo#107815] -> PASS
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-glk: FAIL [fdo#103166] -> PASS
* igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
- shard-apl: DMESG-FAIL [fdo#103166] / [fdo#103558] / [fdo#105602] -> PASS
* igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
- {shard-iclb}: DMESG-WARN [fdo#107724] -> PASS +6
* igt@pm_rpm@modeset-stress-extra-wait:
- shard-skl: INCOMPLETE [fdo#107807] -> PASS
#### Warnings ####
* igt@i915_suspend@shrink:
- shard-skl: INCOMPLETE [fdo#106886] -> DMESG-WARN [fdo#108784]
- shard-kbl: DMESG-WARN [fdo#108784] -> INCOMPLETE [fdo#103665] / [fdo#106886]
* igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
- shard-apl: FAIL [fdo#105458] / [fdo#106510] -> DMESG-WARN [fdo#103558] / [fdo#105602]
* igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
- {shard-iclb}: FAIL [fdo#107725] -> DMESG-WARN [fdo#107724] / [fdo#108336]
* igt@kms_cursor_crc@cursor-128x42-sliding:
- {shard-iclb}: FAIL [fdo#103232] -> DMESG-WARN [fdo#107724] / [fdo#108336]
* igt@kms_cursor_crc@cursor-64x64-suspend:
- {shard-iclb}: FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#107724]
* igt@kms_fbcon_fbt@psr-suspend:
- {shard-iclb}: FAIL [fdo#107882] -> INCOMPLETE [fdo#107713]
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
- {shard-iclb}: FAIL [fdo#103167] -> DMESG-FAIL [fdo#107724]
* igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
- shard-apl: FAIL [fdo#108145] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103257]: https://bugs.freedesktop.org/show_bug.cgi?id=103257
[fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105458]: https://bugs.freedesktop.org/show_bug.cgi?id=105458
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
[fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
[fdo#106887]: https://bugs.freedesktop.org/show_bug.cgi?id=106887
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
[fdo#107931]: https://bugs.freedesktop.org/show_bug.cgi?id=107931
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
[fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
[fdo#108642]: https://bugs.freedesktop.org/show_bug.cgi?id=108642
[fdo#108667]: https://bugs.freedesktop.org/show_bug.cgi?id=108667
[fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_5221 -> Patchwork_10949
CI_DRM_5221: 4c3e98f6787fab0ce3c5d20d25ebd538dfe234af @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4735: b05c028ccdb6ac8e8d8499a041bb14dfe358ee26 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10949: fbec8c5459ca5b586ceb8a3997f9b7df0ea59baa @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10949/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] Return only active connectors for get_resources ioctl
2018-11-29 11:09 [PATCH v3] Return only active connectors for get_resources ioctl Stanislav Lisovskiy
@ 2018-12-04 12:45 ` Jani Nikula
2018-11-29 11:30 ` ✓ Fi.CI.BAT: success for Return only active connectors for get_resources ioctl (rev4) Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2018-12-04 12:45 UTC (permalink / raw)
To: Stanislav Lisovskiy, dri-devel
Cc: ville.syrjala, intel-gfx, martin.peres, stable, stanislav.lisovskiy
On Thu, 29 Nov 2018, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> Currently kernel might allocate different connector ids
> for the same outputs in case of DP MST, which seems to
> confuse userspace. There are can be different connector
> ids in the list, which could be assigned to the same
> output, while being in different states.
> This results in issues, like external displays staying
> blank after quick unplugging and plugging back(bug #106250).
> Returning only active DP connectors fixes the issue.
Some nitpicks here...
The patch subject lacks a prefix.
>
> v2: Removed caps from the title
>
> v3: Refactored initial condition to look more compact.
> Thanks to Chris Wilson for giving a hint.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..3e2cd959a66a 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -143,22 +143,28 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> drm_connector_list_iter_begin(dev, &conn_iter);
> count = 0;
> connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
> + DRM_DEBUG_KMS("GetResources: writing connectors start");
I think the added debugs here are too verbose. You should probably
separate the debugs from the actual change here.
Note also that the logging functions already add the function name, so
"GetResources:" is redundant. Also "\n" is missing.
> drm_for_each_connector_iter(connector, &conn_iter) {
> /* only expose writeback connectors if userspace understands them */
> if (!file_priv->writeback_connectors &&
> (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK))
> continue;
>
> + if (READ_ONCE(connector->registration_state) != DRM_CONNECTOR_REGISTERED)
> + continue;
> +
> if (drm_lease_held(file_priv, connector->base.id)) {
> if (count < card_res->count_connectors &&
> put_user(connector->base.id, connector_id + count)) {
> drm_connector_list_iter_end(&conn_iter);
> return -EFAULT;
> }
> + DRM_DEBUG_KMS("GetResources: connector %s", connector->name);
Please look around, the connector logging is fairly consistent
throughout.
BR,
Jani.
> count++;
> }
> }
> card_res->count_connectors = count;
> + DRM_DEBUG_KMS("GetResources: writing connectors end - count %d", count);
> drm_connector_list_iter_end(&conn_iter);
>
> return ret;
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] Return only active connectors for get_resources ioctl
@ 2018-12-04 12:45 ` Jani Nikula
0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2018-12-04 12:45 UTC (permalink / raw)
To: dri-devel
Cc: ville.syrjala, intel-gfx, martin.peres, stable, stanislav.lisovskiy
On Thu, 29 Nov 2018, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> Currently kernel might allocate different connector ids
> for the same outputs in case of DP MST, which seems to
> confuse userspace. There are can be different connector
> ids in the list, which could be assigned to the same
> output, while being in different states.
> This results in issues, like external displays staying
> blank after quick unplugging and plugging back(bug #106250).
> Returning only active DP connectors fixes the issue.
Some nitpicks here...
The patch subject lacks a prefix.
>
> v2: Removed caps from the title
>
> v3: Refactored initial condition to look more compact.
> Thanks to Chris Wilson for giving a hint.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..3e2cd959a66a 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -143,22 +143,28 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> drm_connector_list_iter_begin(dev, &conn_iter);
> count = 0;
> connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
> + DRM_DEBUG_KMS("GetResources: writing connectors start");
I think the added debugs here are too verbose. You should probably
separate the debugs from the actual change here.
Note also that the logging functions already add the function name, so
"GetResources:" is redundant. Also "\n" is missing.
> drm_for_each_connector_iter(connector, &conn_iter) {
> /* only expose writeback connectors if userspace understands them */
> if (!file_priv->writeback_connectors &&
> (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK))
> continue;
>
> + if (READ_ONCE(connector->registration_state) != DRM_CONNECTOR_REGISTERED)
> + continue;
> +
> if (drm_lease_held(file_priv, connector->base.id)) {
> if (count < card_res->count_connectors &&
> put_user(connector->base.id, connector_id + count)) {
> drm_connector_list_iter_end(&conn_iter);
> return -EFAULT;
> }
> + DRM_DEBUG_KMS("GetResources: connector %s", connector->name);
Please look around, the connector logging is fairly consistent
throughout.
BR,
Jani.
> count++;
> }
> }
> card_res->count_connectors = count;
> + DRM_DEBUG_KMS("GetResources: writing connectors end - count %d", count);
> drm_connector_list_iter_end(&conn_iter);
>
> return ret;
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 8+ messages in thread