[Intel-gfx] [PATCH 13/15] drm/i915: Don't init eDP if we can't find a fixed mode

[Ville Syrjälä]

On Mon, Sep 12, 2022 at 03:02:36PM +0300, Jani Nikula wrote:
> On Mon, 12 Sep 2022, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > In the unlikely case of not finding a fixed mode don't register
> > the eDP connector. I think there are some places where we'd oops
> > if we didn't have a fixed mode for eDP so presumable this doesn't
> > typically happen. But better safe than sorry.
> 
> I think this is fine as the first step. ISTR there are provisions in the
> DP spec for adding some default mode if all else fails, maybe we should
> look into adding something like that?

I have no idea if eDP panels would support even the 640x480
fallback mode. My hunch is no, but might be interesting to
test if someone is willing to risk their eDP panel... 

I have a feeling we should also just remove the current VBT
fixed mode fallback. I think the Windows driver has no such 
thing, though should probably double check that.

> 
> Guaranteeing we always have a fixed mode for eDP opens up possibilities
> for some further cleanup if we want. We have some "is edp and fixed
> mode" style conditions.

Yeah. Though I'm not sure if we'd want to go with
if (edp) or if (fixed_mode) in those cases. I have
occasionally pondered about exposing a user configurable
fixed_mode for external displays as well. Would let users
run their external display with a specific mode and
use the pfit to scale things instead of relying on the
display having a decent scaler.

> 
> Up to and including this patch in the series,

I think I'll skip it for now.

> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>

Thanks.

> 
> 
> >
> > Also pimp the debugs with the encoder id+name. I think dumping
> > the encoder rather than the connector provides more information
> > here (eg. to match again the port information in the VBT).
> >
> > We can also drop the extra check from intel_edp_add_properties().
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 7b4ffb74c94c..8fe48634eb9d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5197,9 +5197,6 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
> >  
> >  	intel_attach_scaling_mode_property(&connector->base);
> >  
> > -	if (!fixed_mode)
> > -		return;
> > -
> >  	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> >  						       i915->display.vbt.orientation,
> >  						       fixed_mode->hdisplay,
> > @@ -5272,7 +5269,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >  	if (!has_dpcd) {
> >  		/* if this fails, presume the device is a ghost */
> >  		drm_info(&dev_priv->drm,
> > -			 "failed to retrieve link info, disabling eDP\n");
> > +			 "[ENCODER:%d:%s] failed to retrieve link info, disabling eDP\n",
> > +			 encoder->base.base.id, encoder->base.name);
> >  		goto out_vdd_off;
> >  	}
> >  
> > @@ -5318,6 +5316,13 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >  
> >  	mutex_unlock(&dev->mode_config.mutex);
> >  
> > +	if (!intel_panel_preferred_fixed_mode(intel_connector)) {
> > +		drm_info(&dev_priv->drm,
> > +			 "[ENCODER:%d:%s] failed to find fixed mode for the panel, disabling eDP\n",
> > +			 encoder->base.base.id, encoder->base.name);
> > +		goto out_vdd_off;
> > +	}
> > +
> >  	intel_panel_init(intel_connector);
> >  
> >  	intel_edp_backlight_setup(intel_dp, intel_connector);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel