diff mbox series

drm/i915/sdvo: Fallback to current output timings for LVDS fixed mode

Message ID 20221025165938.17264-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/sdvo: Fallback to current output timings for LVDS fixed mode | expand

Commit Message

Ville Syrjälä Oct. 25, 2022, 4:59 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we can't dig out a fixed mode for LVDS from the VBT or EDID
let's fall back to using the current output timings. This should
work as long as the BIOS has (somehow) enabled the output.

In this case we are dealing with the some kind of BLB based POS
machine (Toshiba SurePOS 500) where neither the OpRegion mailbox
nor the vbios ROM contain a valid VBT. And no EDID anywhere we
could find either.

Cc: <stable@vger.kernel.org> # v5.19+
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_panel.c |  6 ++--
 drivers/gpu/drm/i915/display/intel_panel.h |  3 ++
 drivers/gpu/drm/i915/display/intel_sdvo.c  | 40 ++++++++++++++++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

Comments

Jani Nikula Oct. 25, 2022, 5:47 p.m. UTC | #1 
On Tue, 25 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we can't dig out a fixed mode for LVDS from the VBT or EDID
> let's fall back to using the current output timings. This should
> work as long as the BIOS has (somehow) enabled the output.
>
> In this case we are dealing with the some kind of BLB based POS
> machine (Toshiba SurePOS 500) where neither the OpRegion mailbox
> nor the vbios ROM contain a valid VBT. And no EDID anywhere we
> could find either.
>
> Cc: <stable@vger.kernel.org> # v5.19+
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

But they're saying it's a regression between 4.19 and 5.10...


> ---
>  drivers/gpu/drm/i915/display/intel_panel.c |  6 ++--
>  drivers/gpu/drm/i915/display/intel_panel.h |  3 ++
>  drivers/gpu/drm/i915/display/intel_sdvo.c  | 40 ++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 69ce77711b7c..69082fbc7647 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -275,9 +275,9 @@ void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
>  	intel_panel_destroy_probed_modes(connector);
>  }
>  
> -static void intel_panel_add_fixed_mode(struct intel_connector *connector,
> -				       struct drm_display_mode *fixed_mode,
> -				       const char *type)
> +void intel_panel_add_fixed_mode(struct intel_connector *connector,
> +				struct drm_display_mode *fixed_mode,
> +				const char *type)
>  {
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct drm_display_info *info = &connector->base.display_info;
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
> index 5c5b5b7f95b6..964efed8ef3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -43,6 +43,9 @@ int intel_panel_fitting(struct intel_crtc_state *crtc_state,
>  			const struct drm_connector_state *conn_state);
>  int intel_panel_compute_config(struct intel_connector *connector,
>  			       struct drm_display_mode *adjusted_mode);
> +void intel_panel_add_fixed_mode(struct intel_connector *connector,
> +				struct drm_display_mode *fixed_mode,
> +				const char *type);
>  void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
>  				      bool use_alt_fixed_modes);
>  void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index cf8e80936d8e..9ed54118b669 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -781,6 +781,13 @@ static bool intel_sdvo_get_input_timing(struct intel_sdvo *intel_sdvo,
>  				     SDVO_CMD_GET_INPUT_TIMINGS_PART1, dtd);
>  }
>  
> +static bool intel_sdvo_get_output_timing(struct intel_sdvo *intel_sdvo,
> +					 struct intel_sdvo_dtd *dtd)
> +{
> +	return intel_sdvo_get_timing(intel_sdvo,
> +				     SDVO_CMD_GET_OUTPUT_TIMINGS_PART1, dtd);
> +}
> +
>  static bool
>  intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
>  					 struct intel_sdvo_connector *intel_sdvo_connector,
> @@ -2864,6 +2871,36 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
>  	return true;
>  }
>  
> +static void
> +intel_sdvo_add_current_fixed_mode(struct intel_sdvo *intel_sdvo,
> +				  struct intel_sdvo_connector *connector)
> +{
> +	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> +	struct drm_display_mode *mode;
> +	struct intel_sdvo_dtd dtd = {};
> +
> +	if (!intel_sdvo_set_target_output(intel_sdvo,
> +					  connector->output_flag)) {
> +		drm_dbg_kms(&i915->drm, "failed to set SDVO target output\n");
> +		return;
> +	}
> +
> +	if (!intel_sdvo_get_output_timing(intel_sdvo, &dtd)) {
> +		drm_dbg_kms(&i915->drm, "failed to get SDVO output timings\n");
> +		return;
> +	}
> +
> +	mode = drm_mode_create(&i915->drm);
> +	if (!mode)
> +		return;
> +
> +	intel_sdvo_get_mode_from_dtd(mode, &dtd);
> +
> +	drm_mode_set_name(mode);
> +
> +	intel_panel_add_fixed_mode(&connector->base, mode, "current (SDVO)");
> +}
> +
>  static bool
>  intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  {
> @@ -2913,6 +2950,9 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  		intel_panel_add_edid_fixed_modes(intel_connector, false);
>  	}
>  
> +	if (!intel_panel_preferred_fixed_mode(intel_connector))
> +		intel_sdvo_add_current_fixed_mode(intel_sdvo, intel_sdvo_connector);
> +
>  	intel_panel_init(intel_connector);
>  
>  	if (!intel_panel_preferred_fixed_mode(intel_connector))
Ville Syrjälä Oct. 25, 2022, 8:32 p.m. UTC | #2 
On Tue, Oct 25, 2022 at 08:47:32PM +0300, Jani Nikula wrote:
> On Tue, 25 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > If we can't dig out a fixed mode for LVDS from the VBT or EDID
> > let's fall back to using the current output timings. This should
> > work as long as the BIOS has (somehow) enabled the output.
> >
> > In this case we are dealing with the some kind of BLB based POS
> > machine (Toshiba SurePOS 500) where neither the OpRegion mailbox
> > nor the vbios ROM contain a valid VBT. And no EDID anywhere we
> > could find either.
> >
> > Cc: <stable@vger.kernel.org> # v5.19+
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> But they're saying it's a regression between 4.19 and 5.10...

Yeah. I can't actually figure out how it could have worked even
with 4.19.

Hmm. Actually now that I look at some of the hints in the logs it
does look like it maybe did find an EDID after all. What confused
me was that all the modes very much like the _noedid stuff.

Ah, it looks like we fail to fully initialize the DDC stuff
before setting up the outputs, which I guess explains why the
EDID read fails there. Previously there was this funky feedback
loop in that .get_modes() actually filled in the fixed_mode,
so until you called that (after everything else was fully set
up) you didn't have a fixed mode.

And while looking at this stuff more I can see that the whole
multi output support is still very much snafu :/

I'll see if I can make a fairly minimal fix for now...
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 69ce77711b7c..69082fbc7647 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -275,9 +275,9 @@  void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
 	intel_panel_destroy_probed_modes(connector);
 }
 
-static void intel_panel_add_fixed_mode(struct intel_connector *connector,
-				       struct drm_display_mode *fixed_mode,
-				       const char *type)
+void intel_panel_add_fixed_mode(struct intel_connector *connector,
+				struct drm_display_mode *fixed_mode,
+				const char *type)
 {
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct drm_display_info *info = &connector->base.display_info;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index 5c5b5b7f95b6..964efed8ef3c 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -43,6 +43,9 @@  int intel_panel_fitting(struct intel_crtc_state *crtc_state,
 			const struct drm_connector_state *conn_state);
 int intel_panel_compute_config(struct intel_connector *connector,
 			       struct drm_display_mode *adjusted_mode);
+void intel_panel_add_fixed_mode(struct intel_connector *connector,
+				struct drm_display_mode *fixed_mode,
+				const char *type);
 void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
 				      bool use_alt_fixed_modes);
 void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index cf8e80936d8e..9ed54118b669 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -781,6 +781,13 @@  static bool intel_sdvo_get_input_timing(struct intel_sdvo *intel_sdvo,
 				     SDVO_CMD_GET_INPUT_TIMINGS_PART1, dtd);
 }
 
+static bool intel_sdvo_get_output_timing(struct intel_sdvo *intel_sdvo,
+					 struct intel_sdvo_dtd *dtd)
+{
+	return intel_sdvo_get_timing(intel_sdvo,
+				     SDVO_CMD_GET_OUTPUT_TIMINGS_PART1, dtd);
+}
+
 static bool
 intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
 					 struct intel_sdvo_connector *intel_sdvo_connector,
@@ -2864,6 +2871,36 @@  intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
 	return true;
 }
 
+static void
+intel_sdvo_add_current_fixed_mode(struct intel_sdvo *intel_sdvo,
+				  struct intel_sdvo_connector *connector)
+{
+	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
+	struct drm_display_mode *mode;
+	struct intel_sdvo_dtd dtd = {};
+
+	if (!intel_sdvo_set_target_output(intel_sdvo,
+					  connector->output_flag)) {
+		drm_dbg_kms(&i915->drm, "failed to set SDVO target output\n");
+		return;
+	}
+
+	if (!intel_sdvo_get_output_timing(intel_sdvo, &dtd)) {
+		drm_dbg_kms(&i915->drm, "failed to get SDVO output timings\n");
+		return;
+	}
+
+	mode = drm_mode_create(&i915->drm);
+	if (!mode)
+		return;
+
+	intel_sdvo_get_mode_from_dtd(mode, &dtd);
+
+	drm_mode_set_name(mode);
+
+	intel_panel_add_fixed_mode(&connector->base, mode, "current (SDVO)");
+}
+
 static bool
 intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 {
@@ -2913,6 +2950,9 @@  intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 		intel_panel_add_edid_fixed_modes(intel_connector, false);
 	}
 
+	if (!intel_panel_preferred_fixed_mode(intel_connector))
+		intel_sdvo_add_current_fixed_mode(intel_sdvo, intel_sdvo_connector);
+
 	intel_panel_init(intel_connector);
 
 	if (!intel_panel_preferred_fixed_mode(intel_connector))