On Wed, 29 Jan 2025, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> The prefix of the mtl_ddi_enable_d2d() / mtl_ddi_disable_d2d_link()
> names show already what are the relevant platforms, so the corresponding
> platform check is a detail that can be hidden in the functions, do so.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
But unrelated bafflement below.
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index d040558b5d029..07188606a0177 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2545,6 +2545,9 @@ mtl_ddi_enable_d2d(struct intel_encoder *encoder)
> i915_reg_t reg;
> u32 set_bits, wait_bits;
>
> + if (DISPLAY_VER(dev_priv) < 14)
> + return;
> +
> if (DISPLAY_VER(dev_priv) >= 20) {
> reg = DDI_BUF_CTL(port);
Unrelated to this patch, this seems weird. DDI_BUF_CTL for display
version 20+? While XELPDP_PORT_BUF_CTL1() also has a check for >= 20.
> set_bits = XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
> @@ -3035,6 +3038,9 @@ mtl_ddi_disable_d2d_link(struct intel_encoder *encoder)
> i915_reg_t reg;
> u32 clr_bits, wait_bits;
>
> + if (DISPLAY_VER(dev_priv) < 14)
> + return;
> +
> if (DISPLAY_VER(dev_priv) >= 20) {
> reg = DDI_BUF_CTL(port);
> clr_bits = XE2LPD_DDI_BUF_D2D_LINK_ENABLE;
> @@ -3411,8 +3417,7 @@ static void intel_ddi_enable_hdmi(struct intel_atomic_state *state,
> hsw_prepare_hdmi_ddi_buffers(encoder, crtc_state);
>
> /* e. Enable D2D Link for C10/C20 Phy */
> - if (DISPLAY_VER(dev_priv) >= 14)
> - mtl_ddi_enable_d2d(encoder);
> + mtl_ddi_enable_d2d(encoder);
Why do we have mtl_ddi_enable_d2d() and mtl_ddi_disable_d2d_link(). It
just boggles my mind that the names mismatch.
>
> encoder->set_signal_levels(encoder, crtc_state);
--
Jani Nikula, Intel
