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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 12 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@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?

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.

Up to and including this patch in the series,

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


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