[Intel-gfx] [PATCH] drm/i915/ehl: Check VBT before updating the transcoder for pipe

[Souza, Jose]

I guess a better solution would be do the HW state sanitization during
the HW readout(intel_modeset_setup_hw_state())

On Tue, 2020-02-04 at 14:44 -0800, Vivek Kasireddy wrote:
> On Tue, 4 Feb 2020 12:50:25 +0200
> Jani Nikula <jani.nikula at linux.intel.com> wrote:
> Hi Jani,
> 
> > On Mon, 03 Feb 2020, Vivek Kasireddy <vivek.kasireddy at intel.com>
> > wrote:
> > > Since the pipe->transcoder mapping is not expected to change
> > > unless
> > > there is either eDP or DSI connectors present, check the VBT to
> > > confirm their presence in addition to checking
> > > TRANS_DDI_FUNC_CTL(transcoder). This additional check is needed
> > > on
> > > platforms like Elkhart Lake because we cannot just rely on
> > > GOP/Firmware programmed values in TRANS_DDI_FUNC_CTL(transcoder)
> > > before updating the transcoder mapping.
> > > 
> > > This patch is only relevant to EHL -- and a no-op on others --
> > > because some of the PHYs are shared between the different DDIs
> > > and
> > > we rely on the VBT to present the most accurate information to
> > > the
> > > driver.
> > > 
> > > Cc: Matt Roper <matthew.d.roper at intel.com>
> > > Cc: José Roberto de Souza <jose.souza at intel.com>
> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 15
> > > ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c index
> > > c0e5002ce64c..4b38f293bd88 100644 ---
> > > a/drivers/gpu/drm/i915/display/intel_display.c +++
> > > b/drivers/gpu/drm/i915/display/intel_display.c @@ -10805,6
> > > +10805,18 @@ static void hsw_get_ddi_pll(struct drm_i915_private
> > > *dev_priv, enum port port, pipe_config->shared_dpll =
> > > intel_get_shared_dpll_by_id(dev_priv, id); } 
> > > +static bool ehl_vbt_edp_dsi_present(struct drm_i915_private
> > > *dev_priv,
> > > +				    enum transcoder transcoder)
> > > +{
> > > +	bool edp_present = intel_bios_is_port_present(dev_priv,
> > > PORT_A);
> > > +	bool dsi_present = intel_bios_is_dsi_present(dev_priv,
> > > NULL); +
> > > +	if (IS_ELKHARTLAKE(dev_priv))
> > > +		return transcoder == TRANSCODER_EDP ? edp_present
> > > : dsi_present; +
> > > +	return true;
> > > +}  
> > 
> > One of those things... this jumps out and immediately feels all
> > wrong,
> > just like ehl_vbt_ddi_d_present() feels all wrong in
> > intel_combo_phy.c. But I don't know what would be the right thing
> > to
> > do without spending time that I don't have on this.
> 
> Is there a particular approach you want me to take to address this
> issue? All I am trying to do is address the plausible scenario(s)
> where
> the GOP/firmware may program the hardware in a certain way that seems
> incorrect from what i915 does based on the info in the VBT. I
> noticed 
> this issue on the EHL board I am working on; therefore, I limited the
> fix to EHL only.
> 
> Thanks,
> Vivek 
> 
> > BR,
> > Jani.
> > 
> > 
> > 
> > > +
> > >  static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> > >  				     struct intel_crtc_state
> > > *pipe_config, u64 *power_domain_mask,
> > > @@ -10844,7 +10856,8 @@ static bool
> > > hsw_get_transcoder_state(struct
> > > intel_crtc *crtc, 
> > >  		tmp = intel_de_read(dev_priv,
> > >  				    TRANS_DDI_FUNC_CTL(panel_transcoder
> > > ));
> > > -		if (!(tmp & TRANS_DDI_FUNC_ENABLE))
> > > +		if (!(tmp & TRANS_DDI_FUNC_ENABLE) ||
> > > +		    !ehl_vbt_edp_dsi_present(dev_priv,
> > > panel_transcoder)) continue;
> > >  
> > >  		/*  
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx