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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jun 08, 2016 at 02:25:25PM +0100, Chris Wilson wrote:
> 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.

Sounds sane. Or we could just kill the entire function and just open
code it. Not sure hiding it in a function buys us anything at this
point.

> 
> 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.

I've been a bit wary about BIT() on account of the UL in there. But I
suppose it might not make much of a difference in the end.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx