* [PATCH] drm: Fix getconnector connection_mutex locking
@ 2014-06-05 9:12 Daniel Vetter
2014-06-05 14:09 ` Rob Clark
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Vetter @ 2014-06-05 9:12 UTC (permalink / raw)
To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Dave Airlie
I've fumbled my own idea and enthusiastically wrapped all the
getconnector code with the connection_mutex. But we only need it to
chase the connector->encoder link. Even there it's not really needed
since races with userspace won't matter, but better paranoid and
consistent about this stuff.
If we grap it everywhere connector probe callbacks can't grab it
themselves, which means they'll deadlock. i915 does that for the load
detect pipe. Furthermore i915 needs to do a ww dance since we also
need to grab the mutex of the load detect crtc.
This is a regression from
commit 6e9f798d91c526982cca0026cd451e8fdbf18aaf
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu May 29 23:54:47 2014 +0200
drm: Split connection_mutex out of mode_config.mutex (v3)
Cc: Rob Clark <robdclark@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_crtc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b8cc4fe0a0f7..fe94cc10cd35 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1838,7 +1838,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
DRM_DEBUG_KMS("[CONNECTOR:%d:?]\n", out_resp->connector_id);
mutex_lock(&dev->mode_config.mutex);
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
connector = drm_connector_find(dev, out_resp->connector_id);
if (!connector) {
@@ -1872,10 +1871,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
out_resp->mm_height = connector->display_info.height_mm;
out_resp->subpixel = connector->display_info.subpixel_order;
out_resp->connection = connector->status;
+ drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
if (connector->encoder)
out_resp->encoder_id = connector->encoder->base.id;
else
out_resp->encoder_id = 0;
+ drm_modeset_unlock(&dev->mode_config.connection_mutex);
/*
* This ioctl is called twice, once to determine how much space is
@@ -1937,7 +1938,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
out_resp->count_encoders = encoders_count;
out:
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
mutex_unlock(&dev->mode_config.mutex);
return ret;
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm: Fix getconnector connection_mutex locking
2014-06-05 9:12 [PATCH] drm: Fix getconnector connection_mutex locking Daniel Vetter
@ 2014-06-05 14:09 ` Rob Clark
0 siblings, 0 replies; 2+ messages in thread
From: Rob Clark @ 2014-06-05 14:09 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Dave Airlie, Intel Graphics Development, DRI Development
On Thu, Jun 5, 2014 at 5:12 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> I've fumbled my own idea and enthusiastically wrapped all the
> getconnector code with the connection_mutex. But we only need it to
> chase the connector->encoder link. Even there it's not really needed
> since races with userspace won't matter, but better paranoid and
> consistent about this stuff.
>
> If we grap it everywhere connector probe callbacks can't grab it
> themselves, which means they'll deadlock. i915 does that for the load
> detect pipe. Furthermore i915 needs to do a ww dance since we also
> need to grab the mutex of the load detect crtc.
>
> This is a regression from
>
> commit 6e9f798d91c526982cca0026cd451e8fdbf18aaf
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Thu May 29 23:54:47 2014 +0200
>
> drm: Split connection_mutex out of mode_config.mutex (v3)
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Rob Clark <robdclark@gmail.com>
> ---
> drivers/gpu/drm/drm_crtc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b8cc4fe0a0f7..fe94cc10cd35 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1838,7 +1838,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> DRM_DEBUG_KMS("[CONNECTOR:%d:?]\n", out_resp->connector_id);
>
> mutex_lock(&dev->mode_config.mutex);
> - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>
> connector = drm_connector_find(dev, out_resp->connector_id);
> if (!connector) {
> @@ -1872,10 +1871,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> out_resp->mm_height = connector->display_info.height_mm;
> out_resp->subpixel = connector->display_info.subpixel_order;
> out_resp->connection = connector->status;
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> if (connector->encoder)
> out_resp->encoder_id = connector->encoder->base.id;
> else
> out_resp->encoder_id = 0;
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
>
> /*
> * This ioctl is called twice, once to determine how much space is
> @@ -1937,7 +1938,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> out_resp->count_encoders = encoders_count;
>
> out:
> - drm_modeset_unlock(&dev->mode_config.connection_mutex);
> mutex_unlock(&dev->mode_config.mutex);
>
> return ret;
> --
> 2.0.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-06-05 14:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 9:12 [PATCH] drm: Fix getconnector connection_mutex locking Daniel Vetter
2014-06-05 14:09 ` Rob Clark
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.