diff mbox

drm: simplify the locking in the GETCRTC ioctl

Message ID 20170328070145.21520-1-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 28, 2017, 7:01 a.m. UTC 
No need to grab both plane and crtc locks at the same time, we can do
them one after the other. If userspace races it'll get what it
deserves either way.

This removes another user of drm_modeset_lock_crtc. There's only one
left.

v2: Make sure all access to primary->state is properly protected
(Harry).

Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc.c           | 11 ++++++++---
 drivers/gpu/drm/i915/intel_display.c |  5 +++++
 2 files changed, 13 insertions(+), 3 deletions(-)

Comments

Harry Wentland March 28, 2017, 2:41 p.m. UTC | #1 
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

On 2017-03-28 03:01 AM, Daniel Vetter wrote:
> No need to grab both plane and crtc locks at the same time, we can do
> them one after the other. If userspace races it'll get what it
> deserves either way.
>
> This removes another user of drm_modeset_lock_crtc. There's only one
> left.
>
> v2: Make sure all access to primary->state is properly protected
> (Harry).
>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c           | 11 ++++++++---
>  drivers/gpu/drm/i915/intel_display.c |  5 +++++
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 660b4c8715de..55b3da2e2a82 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -406,9 +406,9 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	if (!crtc)
>  		return -ENOENT;
>
> -	drm_modeset_lock_crtc(crtc, crtc->primary);
>  	crtc_resp->gamma_size = crtc->gamma_size;
>
> +	drm_modeset_lock(&crtc->primary->mutex, NULL);
>  	if (crtc->primary->state && crtc->primary->state->fb)
>  		crtc_resp->fb_id = crtc->primary->state->fb->base.id;
>  	else if (!crtc->primary->state && crtc->primary->fb)
> @@ -416,9 +416,14 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	else
>  		crtc_resp->fb_id = 0;
>
> -	if (crtc->state) {
> +	if (crtc->primary->state) {
>  		crtc_resp->x = crtc->primary->state->src_x >> 16;
>  		crtc_resp->y = crtc->primary->state->src_y >> 16;
> +	}
> +	drm_modeset_unlock(&crtc->primary->mutex);
> +
> +	drm_modeset_lock(&crtc->mutex, NULL);
> +	if (crtc->state) {
>  		if (crtc->state->enable) {
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
>  			crtc_resp->mode_valid = 1;
> @@ -437,7 +442,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  			crtc_resp->mode_valid = 0;
>  		}
>  	}
> -	drm_modeset_unlock_crtc(crtc);
> +	drm_modeset_unlock(&crtc->mutex);
>
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 68cded453882..43dbad62786e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12463,6 +12463,11 @@ static int intel_atomic_check(struct drm_device *dev,
>  	ret = drm_atomic_helper_check_modeset(dev, state);
>  	if (ret)
>  		return ret;
> +	/* enocder->atomic_check might upgrade some crtc to a full modeset */
> +	ret = drm_atomic_helper_check_modeset(dev, state);
> +	if (ret)
> +		return ret;
> +
>
>  	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) {
>  		struct intel_crtc_state *pipe_config =
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 660b4c8715de..55b3da2e2a82 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -406,9 +406,9 @@  int drm_mode_getcrtc(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
-	drm_modeset_lock_crtc(crtc, crtc->primary);
 	crtc_resp->gamma_size = crtc->gamma_size;
 
+	drm_modeset_lock(&crtc->primary->mutex, NULL);
 	if (crtc->primary->state && crtc->primary->state->fb)
 		crtc_resp->fb_id = crtc->primary->state->fb->base.id;
 	else if (!crtc->primary->state && crtc->primary->fb)
@@ -416,9 +416,14 @@  int drm_mode_getcrtc(struct drm_device *dev,
 	else
 		crtc_resp->fb_id = 0;
 
-	if (crtc->state) {
+	if (crtc->primary->state) {
 		crtc_resp->x = crtc->primary->state->src_x >> 16;
 		crtc_resp->y = crtc->primary->state->src_y >> 16;
+	}
+	drm_modeset_unlock(&crtc->primary->mutex);
+
+	drm_modeset_lock(&crtc->mutex, NULL);
+	if (crtc->state) {
 		if (crtc->state->enable) {
 			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
 			crtc_resp->mode_valid = 1;
@@ -437,7 +442,7 @@  int drm_mode_getcrtc(struct drm_device *dev,
 			crtc_resp->mode_valid = 0;
 		}
 	}
-	drm_modeset_unlock_crtc(crtc);
+	drm_modeset_unlock(&crtc->mutex);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 68cded453882..43dbad62786e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12463,6 +12463,11 @@  static int intel_atomic_check(struct drm_device *dev,
 	ret = drm_atomic_helper_check_modeset(dev, state);
 	if (ret)
 		return ret;
+	/* enocder->atomic_check might upgrade some crtc to a full modeset */
+	ret = drm_atomic_helper_check_modeset(dev, state);
+	if (ret)
+		return ret;
+
 
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) {
 		struct intel_crtc_state *pipe_config =