[Intel-gfx] [PATCH 1/2] drm/i915/dp: Do not set master_trans bit in bitmak if INVALID_TRANSCODER

[Ville Syrjälä]

On Thu, Jan 23, 2020 at 08:31:55PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 23, 2020 at 06:25:29PM +0000, Souza, Jose wrote:
> > On Wed, 2020-01-22 at 16:24 -0800, Manasi Navare wrote:
> > > In the port sync mode, for the master crtc, the master_transcoder is
> > > INVALID.
> > > In that case since its value is -1, do not set the bit in the
> > > bitmask.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Fixes: d0eed1545fe7 ("drm/i915: Fix post-fastset modeset check for
> > > port sync")
> > > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 878d331b9e8c..79f9054078ea 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -14649,8 +14649,10 @@ static int intel_atomic_check(struct
> > > drm_device *dev,
> > >  		}
> > >  
> > >  		if (is_trans_port_sync_mode(new_crtc_state)) {
> > > -			u8 trans = new_crtc_state-
> > > >sync_mode_slaves_mask |
> > > -				   BIT(new_crtc_state-
> > > >master_transcoder);
> > > +			u8 trans = new_crtc_state-
> > > >sync_mode_slaves_mask;
> > > +
> > > +			if (new_crtc_state->master_transcoder !=
> > > INVALID_TRANSCODER)
> > > +				trans |= BIT(new_crtc_state-
> > > >master_transcoder);
> > 
> > Why not set master_transcoder in port sync master too? Would avoid have
> > this check here and in future other places.
> 
> Not how the hardware works. So would complicate hw readout and
> programming code needlessly.

Hmm. Actually not that much maybe. Readout already has to trawl
everything to populate the slave bitmask. So the change to programming
part would just be something like

+ if (cpu_transcoder != master_transcoder)
	enable port sync mode;

So could maybe be done. Might make state dumps a bit more confusing
though since we definitely do not enable port sync mode on the
master transcoder.

> 
> > 
> > >  
> > >  			if (intel_cpu_transcoders_need_modeset(state,
> > > trans)) {
> > >  				new_crtc_state->uapi.mode_changed =
> > > true;
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel