diff mbox

[v3,6/7] drm/atomic: Clean up steal_encoder, v2.

Message ID 1456996662-8704-7-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst March 3, 2016, 9:17 a.m. UTC 
Now that only encoders can be stolen that are part of the state
steal_encoder no longer needs to inspect all connectors,
just those that are part of the atomic state.

Changes since v1:
- Change return value to void, can no longer fail.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

Comments

kernel test robot March 3, 2016, 9:41 a.m. UTC | #1 
Hi Maarten,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20160303]
[cannot apply to v4.5-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-atomic-Fix-encoder-stealing-v3/20160303-172123
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-randconfig-x014-201609 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/drm_atomic_helper.c: In function 'update_connector_routing':
>> drivers/gpu/drm/drm_atomic_helper.c:268:11: warning: unused variable 'ret' [-Wunused-variable]
     int idx, ret;
              ^

vim +/ret +268 drivers/gpu/drm/drm_atomic_helper.c

c6c869e2 Maarten Lankhorst 2016-03-03  252  
c6c869e2 Maarten Lankhorst 2016-03-03  253  		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
c6c869e2 Maarten Lankhorst 2016-03-03  254  		crtc_state->connectors_changed = true;
c6c869e2 Maarten Lankhorst 2016-03-03  255  
49bf38f8 Maarten Lankhorst 2016-03-03  256  		return;
623369e5 Daniel Vetter     2014-09-16  257  	}
623369e5 Daniel Vetter     2014-09-16  258  }
623369e5 Daniel Vetter     2014-09-16  259  
623369e5 Daniel Vetter     2014-09-16  260  static int
184a4cc1 Maarten Lankhorst 2016-03-03  261  update_connector_routing(struct drm_atomic_state *state,
184a4cc1 Maarten Lankhorst 2016-03-03  262  			 struct drm_connector *connector,
184a4cc1 Maarten Lankhorst 2016-03-03  263  			 struct drm_connector_state *connector_state)
623369e5 Daniel Vetter     2014-09-16  264  {
b5ceff20 Ville Syrjälä     2015-03-10  265  	const struct drm_connector_helper_funcs *funcs;
623369e5 Daniel Vetter     2014-09-16  266  	struct drm_encoder *new_encoder;
623369e5 Daniel Vetter     2014-09-16  267  	struct drm_crtc_state *crtc_state;
623369e5 Daniel Vetter     2014-09-16 @268  	int idx, ret;
623369e5 Daniel Vetter     2014-09-16  269  
17a38d9c Daniel Vetter     2015-02-22  270  	DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
623369e5 Daniel Vetter     2014-09-16  271  			 connector->base.id,
623369e5 Daniel Vetter     2014-09-16  272  			 connector->name);
623369e5 Daniel Vetter     2014-09-16  273  
623369e5 Daniel Vetter     2014-09-16  274  	if (connector->state->crtc != connector_state->crtc) {
623369e5 Daniel Vetter     2014-09-16  275  		if (connector->state->crtc) {
623369e5 Daniel Vetter     2014-09-16  276  			idx = drm_crtc_index(connector->state->crtc);

:::::: The code at line 268 was first introduced by commit
:::::: 623369e533e8a5f85999d605723efa4523554bae drm: Atomic crtc/connector updates using crtc/plane helper interfaces

:::::: TO: Daniel Vetter <daniel.vetter@ffwll.ch>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Ville Syrjälä March 4, 2016, 1:27 p.m. UTC | #2 
On Thu, Mar 03, 2016 at 10:17:41AM +0100, Maarten Lankhorst wrote:
> Now that only encoders can be stolen that are part of the state
> steal_encoder no longer needs to inspect all connectors,
> just those that are part of the atomic state.
> 
> Changes since v1:
> - Change return value to void, can no longer fail.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bb60148c5c8d..2395201eb7ab 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -227,25 +227,18 @@ set_best_encoder(struct drm_atomic_state *state,
>  	conn_state->best_encoder = encoder;
>  }
>  
> -static int
> +static void
>  steal_encoder(struct drm_atomic_state *state,
>  	      struct drm_encoder *encoder)
>  {
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_connector *connector;
>  	struct drm_connector_state *connector_state;
> +	int i;
>  
> -	drm_for_each_connector(connector, state->dev) {
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		struct drm_crtc *encoder_crtc;
>  
> -		if (connector->state->best_encoder != encoder)
> -			continue;
> -
> -		connector_state = drm_atomic_get_connector_state(state,
> -								 connector);
> -		if (IS_ERR(connector_state))
> -			return PTR_ERR(connector_state);
> -
>  		if (connector_state->best_encoder != encoder)
>  			continue;
>  
> @@ -260,10 +253,8 @@ steal_encoder(struct drm_atomic_state *state,
>  		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
>  		crtc_state->connectors_changed = true;
>  
> -		return 0;
> +		return;

I was wondering if we should add some kind of sanity check here (or
maybe better do it afterwards across the entire device state?) to
make sure the same encoder didn't end up used multiple times?

Anyway, patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	}
> -
> -	return 0;
>  }
>  
>  static int
> @@ -343,13 +334,7 @@ update_connector_routing(struct drm_atomic_state *state,
>  		return 0;
>  	}
>  
> -	ret = steal_encoder(state, new_encoder);
> -	if (ret) {
> -		DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
> -				 connector->base.id,
> -				 connector->name);
> -		return ret;
> -	}
> +	steal_encoder(state, new_encoder);
>  
>  	if (WARN_ON(!connector_state->crtc))
>  		return -EINVAL;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bb60148c5c8d..2395201eb7ab 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -227,25 +227,18 @@  set_best_encoder(struct drm_atomic_state *state,
 	conn_state->best_encoder = encoder;
 }
 
-static int
+static void
 steal_encoder(struct drm_atomic_state *state,
 	      struct drm_encoder *encoder)
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
+	int i;
 
-	drm_for_each_connector(connector, state->dev) {
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		struct drm_crtc *encoder_crtc;
 
-		if (connector->state->best_encoder != encoder)
-			continue;
-
-		connector_state = drm_atomic_get_connector_state(state,
-								 connector);
-		if (IS_ERR(connector_state))
-			return PTR_ERR(connector_state);
-
 		if (connector_state->best_encoder != encoder)
 			continue;
 
@@ -260,10 +253,8 @@  steal_encoder(struct drm_atomic_state *state,
 		crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
 		crtc_state->connectors_changed = true;
 
-		return 0;
+		return;
 	}
-
-	return 0;
 }
 
 static int
@@ -343,13 +334,7 @@  update_connector_routing(struct drm_atomic_state *state,
 		return 0;
 	}
 
-	ret = steal_encoder(state, new_encoder);
-	if (ret) {
-		DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
-				 connector->base.id,
-				 connector->name);
-		return ret;
-	}
+	steal_encoder(state, new_encoder);
 
 	if (WARN_ON(!connector_state->crtc))
 		return -EINVAL;