Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state

From: Chris Wilson <chris@chris-wilson.co.uk>
To: ville.syrjala@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
Date: Wed, 8 Jun 2016 14:25:25 +0100	[thread overview]
Message-ID: <20160608132525.GE32344@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1465382507-23085-4-git-send-email-ville.syrjala@linux.intel.com>

On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than looping through encoders to see which encoder types
> are being driven by the pipe, add an output_types bitmask into
> the crtc state and populate it prior to compute_config and during
> state readout.
> 
> v2: Determine output_types before .compute_config() hooks are called
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
>  2 files changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 12ff79594bc1..eabace447b1c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
>   */
>  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct intel_encoder *encoder;
> -
> -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> -		if (encoder->type == type)
> -			return true;
> -
> -	return false;
> +	return crtc->config->output_types & (1 << type);

This could become inline and then all those

	if (intel_pipe_has_type(crtc, VGA) ||
	    intel_pipe_has_type(crtc, LVDS) ||
	    intel_pipe_has_type(crtc, HDMI))

whatnots will just disapear into a single test.

I won't say how long those loops have been upsetting me without doing
anything about them.

> @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  			       &pipe_config->pipe_src_w,
>  			       &pipe_config->pipe_src_h);
>  
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != crtc)
> +			continue;
> +
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
> +		/*
> +		 * Determine output_types before calling the .compute_config()
> +		 * hooks so that the hooks can use this information safely.
> +		 */
> +		pipe_config->output_types |= 1 << encoder->type;

pipe_config->output_types |= BIT(encoder->type);

Not my personal favourite in obfuscation, but one available to us if we
desire.

The code looks correct, I can't comment on ordering between computation
of mask and use though.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx