On Tue, Dec 31, 2019 at 01:20:32PM +0200, Jani Nikula wrote:
On Mon, 23 Dec 2019, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
On Mon, Dec 23, 2019 at 11:58:50AM -0800, Lucas De Marchi wrote:
Now that we have tables for all platforms using ddi, keep the port_info
around so we can use it for decisions like "what phy does it have?"
instead of keep checking the platform/gen everywhere.
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 36 ++++++++++++-------
drivers/gpu/drm/i915/display/intel_ddi.h | 8 ++++-
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
.../drm/i915/display/intel_display_types.h | 3 ++
4 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a1b7075ea6be..9d06a34f5f8e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4782,14 +4782,25 @@ intel_ddi_max_lanes(struct intel_digital_port *dig_port)
return max_lanes;
}
-void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
+bool __pure intel_ddi_has_tc_phy(const struct intel_digital_port *dig_port)
{
+ return dig_port->port_info->phy_type == PHY_TYPE_TC;
+}
+
+bool __pure intel_ddi_has_combo_phy(const struct intel_digital_port *dig_port)
+{
+ return dig_port->port_info->phy_type == PHY_TYPE_COMBO;
+}
+
+void intel_ddi_init(struct drm_i915_private *dev_priv,
+ const struct intel_ddi_port_info *port_info)
+{
+ enum port port = port_info->port;
struct ddi_vbt_port_info *vbt_port_info =
&dev_priv->vbt.ddi_port_info[port];
struct intel_digital_port *intel_dig_port;
struct intel_encoder *encoder;
bool init_hdmi, init_dp, init_lspcon = false;
- enum phy phy = intel_port_to_phy(dev_priv, port);
init_hdmi = vbt_port_info->supports_dvi || vbt_port_info->supports_hdmi;
init_dp = vbt_port_info->supports_dp;
@@ -4803,12 +4814,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
init_dp = true;
init_lspcon = true;
init_hdmi = false;
- DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
+ DRM_DEBUG_KMS("VBT says port %s has lspcon\n", port_info->name);
}
if (!init_dp && !init_hdmi) {
- DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n",
- port_name(port));
+ DRM_DEBUG_KMS("VBT says %s is not DVI/HDMI/DP compatible, respect it\n",
+ port_info->name);
return;
}
@@ -4819,7 +4830,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
encoder = &intel_dig_port->base;
drm_encoder_init(&dev_priv->drm, &encoder->base, &intel_ddi_funcs,
- DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
+ DRM_MODE_ENCODER_TMDS, port_info->name);
encoder->hotplug = intel_ddi_hotplug;
encoder->compute_output_type = intel_ddi_compute_output_type;
@@ -4837,7 +4848,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
encoder->type = INTEL_OUTPUT_DDI;
encoder->power_domain = intel_port_to_power_domain(port);
- encoder->port = port;
+ encoder->port = port_info->port;
In theory, shouldn't we be able to drop encoder->port completely once
we've converted everything over to the proper ddi/phy/vbt namespace?
Overall I like the direction this series is going. The continued use of
'port' terminology, both in the driver and in the hardware specs has
become increasingly confusing as things get chopped up and indexed
differently. I think this will help clarify exactly what a platform is
expecting and force people to think about which namespace is correct for
the part of the hardware they're working with.
Indeed, this part I like.
I am less certain whether we need to change output setup to be driven by
the new port_info. Seems like we could keep the existing output setup
(with its wrinkles) while converting port towards a struct consisting of
port, phy and phy type. And make the latter table driven instead of the
current intel_port_to_*() and intel_phy_is_*(). I think that's good
stuff.
I have to put the table somewhere. I prefer it localized on
intel_display then globally on device info because nobody really should
be looking at that table except the init function. This prevents hacks
to sprinkle over the driver.
But I don't like all the wrinkles with port F and DSI and straps etc. in
the output setup changes in this series. And we'll still *also* depend
on VBT here. I am not sure if the output setup should in fact be driven
by the VBT instead of the ports (yeah, *gasp*!).
I think those are orthogonal things. On a quick look, for port F the
thing that can be done is to move it to intel_bios.c as you suggested
and then get rid of the "is_port_present()" hook.
That would allow us to easily use the "generic approach" for gen9lp and
gen11+. As I'm reworking on this stuff, I'd leave the previous platforms
alone so we don't have to make it more complex.
For dsi, it's not clear to me what to do. I could add a
intel_dsi_init(), but that would be just an indirection over the
platform-specific functions. I'm happy to rebase this series on whatever
you provide for dsi.
Lucas De Marchi
I guess the issue with output setup would be if there are collisions in
ports with different phys. Though that would require VBT parsing changes
for DDI too, as that currently ignores such cases (and we have that in
CHV DSI).
BR,
Jani.
Matt
encoder->cloneable = 0;
encoder->pipe_mask = ~0;
@@ -4851,8 +4862,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
intel_dig_port->max_lanes = intel_ddi_max_lanes(intel_dig_port);
intel_dig_port->aux_ch = intel_bios_port_aux_ch(dev_priv, port);
+ intel_dig_port->port_info = port_info;
- if (intel_phy_is_tc(dev_priv, phy)) {
+ if (intel_ddi_has_tc_phy(intel_dig_port)) {
bool is_legacy = !vbt_port_info->supports_typec_usb &&
!vbt_port_info->supports_tbt;
@@ -4883,15 +4895,15 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
if (init_lspcon) {
if (lspcon_init(intel_dig_port))
/* TODO: handle hdmi info frame part */
- DRM_DEBUG_KMS("LSPCON init success on port %c\n",
- port_name(port));
+ DRM_DEBUG_KMS("LSPCON init success on port %s\n",
+ port_info->name);
else
/*
* LSPCON init faied, but DP init was success, so
* lets try to drive as DP++ port.
*/
- DRM_ERROR("LSPCON init failed on port %c\n",
- port_name(port));
+ DRM_ERROR("LSPCON init failed on port %s\n",
+ port_info->name);
}
intel_infoframe_init(intel_dig_port);
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
index 167c6579d972..c500d473963e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.h
+++ b/drivers/gpu/drm/i915/display/intel_ddi.h
@@ -15,6 +15,7 @@ struct drm_i915_private;
struct intel_connector;
struct intel_crtc;
struct intel_crtc_state;
+struct intel_ddi_port_info;
struct intel_dp;
struct intel_dpll_hw_state;
struct intel_encoder;
@@ -24,7 +25,8 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
const struct drm_connector_state *old_conn_state);
void hsw_fdi_link_train(struct intel_encoder *encoder,
const struct intel_crtc_state *crtc_state);
-void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port);
+void intel_ddi_init(struct drm_i915_private *dev_priv,
+ const struct intel_ddi_port_info *port_info);
bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state);
void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state);
@@ -50,4 +52,8 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
struct intel_dpll_hw_state *state);
+
+bool __pure intel_ddi_has_tc_phy(const struct intel_digital_port *dig_port);
+bool __pure intel_ddi_has_combo_phy(const struct intel_digital_port *dig_port);
+
#endif /* __INTEL_DDI_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 219f180fa395..96207dc83fac 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16363,7 +16363,7 @@ static void setup_ddi_outputs(struct drm_i915_private *i915)
!output->is_port_present(i915, port_info))
continue;
- intel_ddi_init(i915, port_info->port);
+ intel_ddi_init(i915, port_info);
}
if (output->dsi_init)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 23a885895803..c54b0178e885 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1346,6 +1346,9 @@ struct intel_digital_port {
enum intel_display_power_domain ddi_io_power_domain;
struct mutex tc_lock; /* protects the TypeC port mode */
intel_wakeref_t tc_lock_wakeref;
+
+ const struct intel_ddi_port_info *port_info;
+
int tc_link_refcount;
bool tc_legacy_port:1;
char tc_port_name[8];
--
2.24.0
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
