| Message ID | 20210402112212.5625-1-contact@emersion.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/connector: demote connector force-probes for non-master clients | expand |
On Fri, 2 Apr 2021 13:22:12 +0200 Simon Ser <contact@emersion.fr> wrote: > Force-probing a connector can be slow and cause flickering. As this > affects the global KMS state, let's make it so only the DRM master > can force-probe a connector. > > Non-master DRM clients won't be able to force-probe a connector > anymore. Instead, KMS will perform a regular read-only connector > query. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> Hi, seems like a good idea to me. Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> Btw. can force-probe be triggered via sysfs as well and does it require root privs? Thanks, pq > --- > drivers/gpu/drm/drm_connector.c | 11 ++++++++--- > include/uapi/drm/drm_mode.h | 7 ++++--- > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 7631f76e7f34..2f70a52a892b 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -20,6 +20,7 @@ > * OF THIS SOFTWARE. > */ > > +#include <drm/drm_auth.h> > #include <drm/drm_connector.h> > #include <drm/drm_edid.h> > #include <drm/drm_encoder.h> > @@ -2374,9 +2375,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > > mutex_lock(&dev->mode_config.mutex); > if (out_resp->count_modes == 0) { > - connector->funcs->fill_modes(connector, > - dev->mode_config.max_width, > - dev->mode_config.max_height); > + if (drm_is_current_master(file_priv)) > + connector->funcs->fill_modes(connector, > + dev->mode_config.max_width, > + dev->mode_config.max_height); > + else > + drm_dbg_kms(dev, "User-space requested a forced probe on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe", > + connector->base.id, connector->name); > } > > out_resp->mm_width = connector->display_info.width_mm; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index a5e76aa06ad5..3efa2e38d89b 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -413,9 +413,10 @@ enum drm_mode_subconnector { > * > * **Force-probing a connector** > * > - * If the @count_modes field is set to zero, the kernel will perform a forced > - * probe on the connector to refresh the connector status, modes and EDID. > - * A forced-probe can be slow, might cause flickering and the ioctl will block. > + * If the @count_modes field is set to zero and the DRM client is the DRM > + * master, the kernel will perform a forced probe on the connector to refresh > + * the connector status, modes and EDID. A forced-probe can be slow, might > + * cause flickering and the ioctl will block. > * > * User-space needs to force-probe connectors to ensure their metadata is > * up-to-date at startup and after receiving a hot-plug event. User-space
On Wednesday, April 7th, 2021 at 9:02 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote: > Btw. can force-probe be triggered via sysfs as well and does it require > root privs? sysfs can force-probe like so: echo detect | sudo tee /sys/class/drm/card0-DP-1/status But this requires root, yes.
On Wed, 07 Apr 2021 07:16:30 +0000 Simon Ser <contact@emersion.fr> wrote: > On Wednesday, April 7th, 2021 at 9:02 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote: > > > Btw. can force-probe be triggered via sysfs as well and does it require > > root privs? > > sysfs can force-probe like so: > > echo detect | sudo tee /sys/class/drm/card0-DP-1/status > > But this requires root, yes. Ah, all good then. Thanks, pq
Ping Daniel Vetter
On Fri, Apr 02, 2021 at 01:22:12PM +0200, Simon Ser wrote: > Force-probing a connector can be slow and cause flickering. As this > affects the global KMS state, let's make it so only the DRM master > can force-probe a connector. > > Non-master DRM clients won't be able to force-probe a connector > anymore. Instead, KMS will perform a regular read-only connector > query. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > --- > drivers/gpu/drm/drm_connector.c | 11 ++++++++--- > include/uapi/drm/drm_mode.h | 7 ++++--- > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 7631f76e7f34..2f70a52a892b 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -20,6 +20,7 @@ > * OF THIS SOFTWARE. > */ > > +#include <drm/drm_auth.h> > #include <drm/drm_connector.h> > #include <drm/drm_edid.h> > #include <drm/drm_encoder.h> > @@ -2374,9 +2375,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, > > mutex_lock(&dev->mode_config.mutex); > if (out_resp->count_modes == 0) { > - connector->funcs->fill_modes(connector, > - dev->mode_config.max_width, > - dev->mode_config.max_height); > + if (drm_is_current_master(file_priv)) > + connector->funcs->fill_modes(connector, > + dev->mode_config.max_width, > + dev->mode_config.max_height); > + else > + drm_dbg_kms(dev, "User-space requested a forced probe on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe", > + connector->base.id, connector->name); > } > > out_resp->mm_width = connector->display_info.width_mm; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index a5e76aa06ad5..3efa2e38d89b 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -413,9 +413,10 @@ enum drm_mode_subconnector { > * > * **Force-probing a connector** > * > - * If the @count_modes field is set to zero, the kernel will perform a forced > - * probe on the connector to refresh the connector status, modes and EDID. > - * A forced-probe can be slow, might cause flickering and the ioctl will block. > + * If the @count_modes field is set to zero and the DRM client is the DRM *current* DRM master The drm master/client relationship survives a DROPMASTER ioctl, but also it's only really relevant for the old authmagic dance. But just to be consistent here. > + * master, the kernel will perform a forced probe on the connector to refresh > + * the connector status, modes and EDID. A forced-probe can be slow, might > + * cause flickering and the ioctl will block. Do we have an igt for this? Timing test should do the job I think, assuming we have at least one output which requires an edid read (so maybe skip the test if a forced probe takes less than 10ms or so). Patch lgtm, but igt would be really nice here. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > * > * User-space needs to force-probe connectors to ensure their metadata is > * up-to-date at startup and after receiving a hot-plug event. User-space > -- > 2.31.1 >
On Tuesday, April 20th, 2021 at 11:14 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > Do we have an igt for this? Timing test should do the job I think, > assuming we have at least one output which requires an edid read (so maybe > skip the test if a forced probe takes less than 10ms or so). Err, an igt that only relies on timings? Sorry, but that sounds like a recipe for flaky tests. Ideally a chamelium test would allow to make sure all of this works as expected. I don't really have this hw anymore though.
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 7631f76e7f34..2f70a52a892b 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -20,6 +20,7 @@ * OF THIS SOFTWARE. */ +#include <drm/drm_auth.h> #include <drm/drm_connector.h> #include <drm/drm_edid.h> #include <drm/drm_encoder.h> @@ -2374,9 +2375,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, mutex_lock(&dev->mode_config.mutex); if (out_resp->count_modes == 0) { - connector->funcs->fill_modes(connector, - dev->mode_config.max_width, - dev->mode_config.max_height); + if (drm_is_current_master(file_priv)) + connector->funcs->fill_modes(connector, + dev->mode_config.max_width, + dev->mode_config.max_height); + else + drm_dbg_kms(dev, "User-space requested a forced probe on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe", + connector->base.id, connector->name); } out_resp->mm_width = connector->display_info.width_mm; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a5e76aa06ad5..3efa2e38d89b 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -413,9 +413,10 @@ enum drm_mode_subconnector { * * **Force-probing a connector** * - * If the @count_modes field is set to zero, the kernel will perform a forced - * probe on the connector to refresh the connector status, modes and EDID. - * A forced-probe can be slow, might cause flickering and the ioctl will block. + * If the @count_modes field is set to zero and the DRM client is the DRM + * master, the kernel will perform a forced probe on the connector to refresh + * the connector status, modes and EDID. A forced-probe can be slow, might + * cause flickering and the ioctl will block. * * User-space needs to force-probe connectors to ensure their metadata is * up-to-date at startup and after receiving a hot-plug event. User-space
Force-probing a connector can be slow and cause flickering. As this affects the global KMS state, let's make it so only the DRM master can force-probe a connector. Non-master DRM clients won't be able to force-probe a connector anymore. Instead, KMS will perform a regular read-only connector query. Signed-off-by: Simon Ser <contact@emersion.fr> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Pekka Paalanen <ppaalanen@gmail.com> --- drivers/gpu/drm/drm_connector.c | 11 ++++++++--- include/uapi/drm/drm_mode.h | 7 ++++--- 2 files changed, 12 insertions(+), 6 deletions(-)