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

[Vivek Kasireddy]

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;
> >  
> >  		/*  
>