diff mbox series

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

Message ID 20240319092443.15769-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/bios: Tolerate devdata==NULL in intel_bios_encoder_supports_dp_dual_mode() | expand

Commit Message

Ville Syrjälä March 19, 2024, 9:24 a.m. UTC
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?

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(-)

Comments

Jani Nikula March 19, 2024, 10:20 a.m. UTC | #1
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;
diff mbox series

Patch

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);
 
 	if (!HAS_DDI(i915) && !IS_CHERRYVIEW(i915))
 		return;