On Mon, Nov 13, 2023 at 06:47:10PM +0200, Jani Nikula wrote: > vlv_dpio_read() and vlv_dpio_write() really operate on the phy, not > pipe. Passing the pipe instead of the phy as parameter is supposed to be > a convenience, but when the caller has the phy, it becomes an > inconvenience. See e.g. chv_dpio_cmn_power_well_enable() and > assert_chv_phy_powergate(). > > Figure out the phy in the callers, and pass phy to the dpio functions. > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > --- > .../i915/display/intel_display_power_well.c | 23 +-- > drivers/gpu/drm/i915/display/intel_dpio_phy.c | 160 +++++++++--------- > drivers/gpu/drm/i915/display/intel_dpll.c | 106 ++++++------ > drivers/gpu/drm/i915/vlv_sideband.c | 10 +- > drivers/gpu/drm/i915/vlv_sideband.h | 6 +- > 5 files changed, 152 insertions(+), 153 deletions(-) <snip> > diff --git a/drivers/gpu/drm/i915/display/intel_dpio_phy.c b/drivers/gpu/drm/i915/display/intel_dpio_phy.c > index d6af46e33424..32886c0ec2cc 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpio_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_dpio_phy.c > @@ -1107,24 +1109,24 @@ void vlv_phy_pre_encoder_enable(struct intel_encoder *encoder, > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > enum dpio_channel port = vlv_dig_port_to_channel(dig_port); > - enum pipe pipe = crtc->pipe; > + enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe); > u32 val; > > vlv_dpio_get(dev_priv); > > /* Enable clock channels for this port */ > - val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW8(port)); > + val = vlv_dpio_read(dev_priv, phy, VLV_PCS01_DW8(port)); > val = 0; > - if (pipe) > + if (phy) That is wrong. Apart from that looks identical to what I have in one of my branches :) With that bogus change dropped: Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> -- Ville Syrjälä Intel