Re: [PATCH 04/11] drm/i915: Add for_each_connector_in_state helper macro

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Apr 10, 2015 at 11:38:33AM +0300, Ander Conselvan de Oliveira wrote:
> This saves some typing whenever a iteration over all the connector
> states in the atomic state is written, which happens quite often.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  8 +++++
>  drivers/gpu/drm/i915/intel_ddi.c     |  9 ++---
>  drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++--------------------------
>  drivers/gpu/drm/i915/intel_dp_mst.c  | 12 +++----
>  drivers/gpu/drm/i915/intel_hdmi.c    |  7 ++--
>  5 files changed, 38 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 78f31cb..e12bc5a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -264,6 +264,14 @@ enum hpd_pin {
>  	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
>  		if ((1 << (domain)) & (mask))
>  
> +#define for_each_connector_in_state(state, connector, connector_state, __i) \

Imo this should be a drm_atomic.h thing. At least there's a lot of loops
with exactly this pattern in drm_atomic_helper.c and drm_atomic.c

Maybe even do helpers for plane/crtc looping too.
-Daniel


> +	for ((__i) = 0;							\
> +	     (connector) = to_intel_connector((state)->connectors[__i]), \
> +	     (connector_state) = (state)->connector_states[__i],	\
> +	     (__i) < (state)->num_connector;				\
> +	     (__i)++)							\
> +		if (connector)
> +
>  struct drm_i915_private;
>  struct i915_mm_struct;
>  struct i915_mmu_object;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 486f6fa..2e005e3 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -496,18 +496,19 @@ intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct intel_encoder *ret = NULL;
> +	struct intel_connector *connector;
>  	struct drm_atomic_state *state;
> +	struct drm_connector_state *connector_state;
>  	int num_encoders = 0;
>  	int i;
>  
>  	state = crtc_state->base.state;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i] ||
> -		    state->connector_states[i]->crtc != crtc_state->base.crtc)
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> -		ret = to_intel_encoder(state->connector_states[i]->best_encoder);
> +		ret = to_intel_encoder(connector_state->best_encoder);
>  		num_encoders++;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 894788d..3035a3d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -437,13 +437,10 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
>  	struct drm_atomic_state *state = crtc_state->base.state;
>  	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	int i, num_connectors = 0;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> @@ -6919,16 +6916,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	bool ok, has_reduced_clock = false;
>  	bool is_lvds = false, is_dsi = false;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	const intel_limit_t *limit;
>  	struct drm_atomic_state *state = crtc_state->base.state;
>  	struct drm_connector_state *connector_state;
>  	int i;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != &crtc->base)
>  			continue;
>  
> @@ -7614,14 +7608,11 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
>  	struct drm_atomic_state *state = crtc_state->base.state;
>  	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	int num_connectors = 0, i;
>  	bool is_lvds = false;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> @@ -7877,15 +7868,12 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct drm_atomic_state *state = crtc_state->base.state;
>  	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	uint32_t dpll;
>  	int factor, num_connectors = 0, i;
>  	bool is_lvds = false, is_sdvo = false;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> @@ -10522,6 +10510,7 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_atomic_state *state;
> +	struct drm_connector_state *connector_state;
>  	struct intel_connector *connector;
>  	int bpp, i;
>  
> @@ -10566,12 +10555,8 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  	state = pipe_config->base.state;
>  
>  	/* Clamp display bpp to EDID value */
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector = to_intel_connector(state->connectors[i]);
> -		if (state->connector_states[i]->crtc != &crtc->base)
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != &crtc->base)
>  			continue;
>  
>  		connected_sink_compute_bpp(connector, pipe_config);
> @@ -10658,14 +10643,11 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
>  					 struct intel_encoder *encoder)
>  {
>  	struct intel_encoder *source_encoder;
> +	struct intel_connector *connector;
>  	struct drm_connector_state *connector_state;
>  	int i;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != &crtc->base)
>  			continue;
>  
> @@ -10682,14 +10664,11 @@ static bool check_encoder_cloning(struct drm_atomic_state *state,
>  				  struct intel_crtc *crtc)
>  {
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	struct drm_connector_state *connector_state;
>  	int i;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != &crtc->base)
>  			continue;
>  
> @@ -10705,6 +10684,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	struct drm_connector_state *connector_state;
>  	unsigned int used_ports = 0;
>  	int i;
> @@ -10714,11 +10694,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  	 * list to detect the problem on ddi platforms
>  	 * where there's just one encoder per digital port.
>  	 */
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (!connector_state->best_encoder)
>  			continue;
>  
> @@ -10845,12 +10821,7 @@ encoder_retry:
>  	 * adjust it according to limitations or connector properties, and also
>  	 * a chance to reject the mode entirely.
>  	 */
> -	for (i = 0; i < state->num_connector; i++) {
> -		connector = to_intel_connector(state->connectors[i]);
> -		if (!connector)
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != crtc)
>  			continue;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 7335089..5561eca 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -40,7 +40,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	int bpp, i;
>  	int lane_count, slots, rate;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	struct intel_connector *found = NULL;
> +	struct intel_connector *connector, *found = NULL;
> +	struct drm_connector_state *connector_state;
>  	int mst_pbn;
>  
>  	pipe_config->dp_encoder_is_mst = true;
> @@ -70,12 +71,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  
>  	state = pipe_config->base.state;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		if (state->connector_states[i]->best_encoder == &encoder->base) {
> -			found = to_intel_connector(state->connectors[i]);
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->best_encoder == &encoder->base) {
> +			found = connector;
>  			break;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 02252d9..9d02746 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -956,6 +956,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_atomic_state *state;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
>  	struct drm_connector_state *connector_state;
>  	int count = 0, count_hdmi = 0;
>  	int i;
> @@ -965,11 +966,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>  
>  	state = crtc_state->base.state;
>  
> -	for (i = 0; i < state->num_connector; i++) {
> -		if (!state->connectors[i])
> -			continue;
> -
> -		connector_state = state->connector_states[i];
> +	for_each_connector_in_state(state, connector, connector_state, i) {
>  		if (connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx