On Tue, 14 Nov 2023, Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:
> 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.nik...@intel.com>
>> ---
>>  .../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 :)

Whoops, over eager replace. Good catch!

>
> With that bogus change dropped:
> Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

Thanks!

-- 
Jani Nikula, Intel

Reply via email to