On Fri, 17 Dec 2021, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Now that we parse the DDI port info from the VBT on all g4x+ platforms
> we can throw out all the old codepaths in intel_bios_is_port_present(),
> intel_bios_is_port_edp() and intel_bios_is_port_dp_dual_mode(). None
> of these should be called on pre-g4x platforms.
>
> For good measure throw in a WARN into intel_bios_is_port_present()
> should someone get the urge to call it on older platforms. The
> other two functions are specific to HDMI and DP so should not need
> any protection as those encoder types don't even exist on older
> platforms.
This patch is one of the main reasons why I'm in favor of taking the
risks. It's just much simpler overall.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
c
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 99 ++-----------------
> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 15 ---
> 2 files changed, 9 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index d7d64d3bf083..f5aa2c72b2fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2663,37 +2663,10 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *i915, u8 *i2c_pin)
> */
> bool intel_bios_is_port_present(struct drm_i915_private *i915, enum port port)
> {
> - const struct intel_bios_encoder_data *devdata;
> - const struct child_device_config *child;
> - static const struct {
> - u16 dp, hdmi;
> - } port_mapping[] = {
> - [PORT_B] = { DVO_PORT_DPB, DVO_PORT_HDMIB, },
> - [PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
> - [PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
> - [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> - [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> - };
> + if (WARN_ON(!has_ddi_port_info(i915)))
> + return true;
>
> - if (has_ddi_port_info(i915))
> - return i915->vbt.ports[port];
> -
> - /* FIXME maybe deal with port A as well? */
> - if (drm_WARN_ON(&i915->drm,
> - port == PORT_A) || port >= ARRAY_SIZE(port_mapping))
> - return false;
> -
> - list_for_each_entry(devdata, &i915->vbt.display_devices, node) {
> - child = &devdata->child;
> -
> - if ((child->dvo_port == port_mapping[port].dp ||
> - child->dvo_port == port_mapping[port].hdmi) &&
> - (child->device_type & (DEVICE_TYPE_TMDS_DVI_SIGNALING |
> - DEVICE_TYPE_DISPLAYPORT_OUTPUT)))
> - return true;
> - }
> -
> - return false;
> + return i915->vbt.ports[port];
> }
>
> /**
> @@ -2705,34 +2678,10 @@ bool intel_bios_is_port_present(struct drm_i915_private *i915, enum port port)
> */
> bool intel_bios_is_port_edp(struct drm_i915_private *i915, enum port port)
> {
> - const struct intel_bios_encoder_data *devdata;
> - const struct child_device_config *child;
> - static const short port_mapping[] = {
> - [PORT_B] = DVO_PORT_DPB,
> - [PORT_C] = DVO_PORT_DPC,
> - [PORT_D] = DVO_PORT_DPD,
> - [PORT_E] = DVO_PORT_DPE,
> - [PORT_F] = DVO_PORT_DPF,
> - };
> + const struct intel_bios_encoder_data *devdata =
> + intel_bios_encoder_data_lookup(i915, port);
>
> - if (has_ddi_port_info(i915)) {
> - const struct intel_bios_encoder_data *devdata;
> -
> - devdata = intel_bios_encoder_data_lookup(i915, port);
> -
> - return devdata && intel_bios_encoder_supports_edp(devdata);
> - }
> -
> - list_for_each_entry(devdata, &i915->vbt.display_devices, node) {
> - child = &devdata->child;
> -
> - if (child->dvo_port == port_mapping[port] &&
> - (child->device_type & DEVICE_TYPE_eDP_BITS) ==
> - (DEVICE_TYPE_eDP & DEVICE_TYPE_eDP_BITS))
> - return true;
> - }
> -
> - return false;
> + return devdata && intel_bios_encoder_supports_edp(devdata);
> }
>
> static bool child_dev_is_dp_dual_mode(const struct child_device_config *child)
> @@ -2755,40 +2704,10 @@ static bool child_dev_is_dp_dual_mode(const struct child_device_config *child)
> bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *i915,
> enum port port)
> {
> - static const struct {
> - u16 dp, hdmi;
> - } port_mapping[] = {
> - /*
> - * Buggy VBTs may declare DP ports as having
> - * HDMI type dvo_port :( So let's check both.
> - */
> - [PORT_B] = { DVO_PORT_DPB, DVO_PORT_HDMIB, },
> - [PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
> - [PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
> - [PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> - [PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
> - };
> - const struct intel_bios_encoder_data *devdata;
> + const struct intel_bios_encoder_data *devdata =
> + intel_bios_encoder_data_lookup(i915, port);
>
> - if (has_ddi_port_info(i915)) {
> - const struct intel_bios_encoder_data *devdata;
> -
> - devdata = intel_bios_encoder_data_lookup(i915, port);
> -
> - return devdata && child_dev_is_dp_dual_mode(&devdata->child);
> - }
> -
> - if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> - return false;
> -
> - list_for_each_entry(devdata, &i915->vbt.display_devices, node) {
> - if ((devdata->child.dvo_port == port_mapping[port].dp ||
> - devdata->child.dvo_port == port_mapping[port].hdmi) &&
> - child_dev_is_dp_dual_mode(&devdata->child))
> - return true;
> - }
> -
> - return false;
> + return devdata && child_dev_is_dp_dual_mode(&devdata->child);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index f043d85ba64d..c23582769f34 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -226,21 +226,6 @@ struct bdb_general_features {
> #define DEVICE_TYPE_DIGITAL_OUTPUT (1 << 1)
> #define DEVICE_TYPE_ANALOG_OUTPUT (1 << 0)
>
> -/*
> - * Bits we care about when checking for DEVICE_TYPE_eDP. Depending on the
> - * system, the other bits may or may not be set for eDP outputs.
> - */
> -#define DEVICE_TYPE_eDP_BITS \
> - (DEVICE_TYPE_INTERNAL_CONNECTOR | \
> - DEVICE_TYPE_MIPI_OUTPUT | \
> - DEVICE_TYPE_COMPOSITE_OUTPUT | \
> - DEVICE_TYPE_DUAL_CHANNEL | \
> - DEVICE_TYPE_LVDS_SIGNALING | \
> - DEVICE_TYPE_TMDS_DVI_SIGNALING | \
> - DEVICE_TYPE_VIDEO_SIGNALING | \
> - DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
> - DEVICE_TYPE_ANALOG_OUTPUT)
> -
> #define DEVICE_TYPE_DP_DUAL_MODE_BITS \
> (DEVICE_TYPE_INTERNAL_CONNECTOR | \
> DEVICE_TYPE_MIPI_OUTPUT | \
--
Jani Nikula, Intel Open Source Graphics Center
