Re: [PATCH 2/2] drm/i915/bios: Use the platform's port_mask when there is no VBT

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

On Tue, 19 Mar 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When we have no VBT we currently assume ports A-F are
> all pontially valid for every platform. That is nonsense.
> Grab the bitmask of valid ports from the runtime info
> instead.
>
> Although the defaults we actually fill here look semi-sensible
> only for hsw-skl era hardware. Dunno if we should try to do
> something more appropriate here for other platforms,
> or just try to nuke the whole thing?

Seems like doing something appropriate is an endless task. We should
never have added the defaults, but here we are. Can we remove them for
newer platforms? There's only one way to find out...

Commit 828ccb31cf41 ("drm/i915/icl: Add TypeC ports only if VBT is
present") fails to mention if this was to address a real issue on ICL
without VBT, or just playing it safe.

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c13a98431a7b..2abd2d7ceda2 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2890,9 +2890,8 @@ init_vbt_panel_defaults(struct intel_panel *panel)
>  static void
>  init_vbt_missing_defaults(struct drm_i915_private *i915)
>  {
> +	unsigned int ports = DISPLAY_RUNTIME_INFO(i915)->port_mask;
>  	enum port port;
> -	int ports = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) |
> -		    BIT(PORT_D) | BIT(PORT_E) | BIT(PORT_F);

I was wondering why we don't just

-#define for_each_port(__port) \
-	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)
+#define for_each_port(__i915, __port)					\
+	for_each_set_bit(__port, &DISPLAY_RUNTIME_INFO(__i915)->port_mask, sizeof(DISPLAY_RUNTIME_INFO(__i915)->port_mask))

but then we have DSI code that relies on it returning PORT_A for DSI.

Ugh. It's all so brittle.

Anyway, the patch is

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

>  
>  	if (!HAS_DDI(i915) && !IS_CHERRYVIEW(i915))
>  		return;

-- 
Jani Nikula, Intel