Re: [Intel-gfx] [PATCH 08/12] drm/i915/vbt: add helper functions to check output support

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Wed, Feb 17, 2021 at 09:55:53PM +0200, Jani Nikula wrote:
>On Wed, 17 Feb 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Wed, Feb 17, 2021 at 07:03:38PM +0200, Jani Nikula wrote:
>>>These will be exposed to the rest of the driver and replace other
>>>functions. Everything will operate on the child devices.
>>>
>>>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>---
>>> drivers/gpu/drm/i915/display/intel_bios.c | 66 ++++++++++++++++++-----
>>> 1 file changed, 54 insertions(+), 12 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>>>index 193772f42582..739ef5d91907 100644
>>>--- a/drivers/gpu/drm/i915/display/intel_bios.c
>>>+++ b/drivers/gpu/drm/i915/display/intel_bios.c
>>>@@ -64,6 +64,7 @@ struct intel_bios_encoder_data {
>>>
>>> 	struct child_device_config child;
>>> 	struct dsc_compression_parameters_entry *dsc;
>>>+
>>> 	struct list_head node;
>>> };
>>>
>>>@@ -1761,6 +1762,53 @@ static enum port dvo_port_to_port(struct drm_i915_private *i915,
>>> 					  dvo_port);
>>> }
>>>
>>>+static void sanitize_device_type(struct intel_bios_encoder_data *devdata,
>>>+				 enum port port)
>>>+{
>>>+	struct drm_i915_private *i915 = devdata->i915;
>>>+	bool is_hdmi;
>>>+
>>>+	if (port != PORT_A || INTEL_GEN(i915) >= 12)
>>>+		return;
>>>+
>>>+	if (!(devdata->child.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING))
>>>+		return;
>>>+
>>>+	is_hdmi = !(devdata->child.device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT);
>>>+
>>>+	drm_dbg_kms(&i915->drm, "VBT claims port A supports DVI%s, ignoring\n",
>>>+		    is_hdmi ? "/HDMI" : "");
>>>+
>>>+	devdata->child.device_type &= ~DEVICE_TYPE_TMDS_DVI_SIGNALING;
>>>+	devdata->child.device_type |= DEVICE_TYPE_NOT_HDMI_OUTPUT;
>>>+}
>>>+
>>>+static bool
>>>+intel_bios_encoder_supports_dvi(const struct intel_bios_encoder_data *devdata)
>>>+{
>>>+	return devdata->child.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
>>>+}
>>>+
>>>+static bool
>>>+intel_bios_encoder_supports_hdmi(const struct intel_bios_encoder_data *devdata)
>>>+{
>>>+	return intel_bios_encoder_supports_dvi(devdata) &&
>>>+		(devdata->child.device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0;
>>>+}
>>>+
>>>+static bool
>>>+intel_bios_encoder_supports_dp(const struct intel_bios_encoder_data *devdata)
>>>+{
>>>+	return devdata->child.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
>>>+}
>>>+
>>>+static bool
>>>+intel_bios_encoder_supports_edp(const struct intel_bios_encoder_data *devdata)
>>>+{
>>>+	return intel_bios_encoder_supports_dp(devdata) &&
>>>+		devdata->child.device_type & DEVICE_TYPE_INTERNAL_CONNECTOR;
>>>+}
>>>+
>>> static void parse_ddi_port(struct drm_i915_private *i915,
>>> 			   struct intel_bios_encoder_data *devdata)
>>> {
>>>@@ -1782,19 +1830,13 @@ static void parse_ddi_port(struct drm_i915_private *i915,
>>> 		return;
>>> 	}
>>>
>>>-	is_dvi = child->device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
>>>-	is_dp = child->device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
>>>-	is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT;
>>>-	is_hdmi = is_dvi && (child->device_type & DEVICE_TYPE_NOT_HDMI_OUTPUT) == 0;
>>>-	is_edp = is_dp && (child->device_type & DEVICE_TYPE_INTERNAL_CONNECTOR);
>>>+	sanitize_device_type(devdata, port);
>>>
>>>-	if (port == PORT_A && is_dvi && INTEL_GEN(i915) < 12) {
>>>-		drm_dbg_kms(&i915->drm,
>>>-			    "VBT claims port A supports DVI%s, ignoring\n",
>>>-			    is_hdmi ? "/HDMI" : "");
>>>-		is_dvi = false;
>>>-		is_hdmi = false;
>>>-	}
>>>+	is_dvi = intel_bios_encoder_supports_dvi(devdata);
>>>+	is_dp = intel_bios_encoder_supports_dp(devdata);
>>>+	is_crt = child->device_type & DEVICE_TYPE_ANALOG_OUTPUT;
>>
>> any reason to leave this behind?
>
>Just logging, even though that doesn't match what we actually do in
>intel_ddi_crt_present(). I expect there to be further cleanup
>afterwards.

I mean: you added a intel_bios_encoder_supports_*() for everything else.
Why not for crt?

Lucas De Marchi

>
>BR,
>Jani.
>
>>
>> Lucas De Marchi
>>
>>>+	is_hdmi = intel_bios_encoder_supports_hdmi(devdata);
>>>+	is_edp = intel_bios_encoder_supports_edp(devdata);
>>>
>>> 	info->supports_dvi = is_dvi;
>>> 	info->supports_hdmi = is_hdmi;
>>>--
>>>2.20.1
>>>
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx