On Mon, 22 Apr 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> In the encoder hooks we are dealing primarily with the encoder,
> so derive the DPIO PHY from the encoder rather than the pipe.
> Technically this doesn't matter as we can't cross connect
> pipes<->port across PHY boundaries, but it does conveny the
> intention more accurately.
I'll note that for most places converting vlv_dig_port_to_* to
vlv_encoder_to_* would be more convenient in the caller side. We have
the encoder available where they're needed, and in many places we use
enc_to_dig_port(encoder) just to be able to use vlv_dig_port_to_*.
I'd just convert them to vlv_encoder_to_*.
Regardless, this does what it says on the tin,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Oh, one comment inline near the end.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dpio_phy.c | 27 ++++++++-----------
> drivers/gpu/drm/i915/vlv_sideband.c | 1 -
> 2 files changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpio_phy.c b/drivers/gpu/drm/i915/display/intel_dpio_phy.c
> index e4a04c9b5b19..4fafac534967 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpio_phy.c
> @@ -719,9 +719,8 @@ void chv_set_phy_signal_level(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> enum dpio_channel ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
> u32 val;
> int i;
>
> @@ -814,9 +813,9 @@ void chv_data_lane_soft_reset(struct intel_encoder *encoder,
> bool reset)
> {
> 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 ch = vlv_dig_port_to_channel(enc_to_dig_port(encoder));
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> + enum dpio_channel ch = vlv_dig_port_to_channel(dig_port);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
> u32 val;
>
> val = vlv_dpio_read(dev_priv, phy, VLV_PCS01_DW0(ch));
> @@ -861,7 +860,7 @@ void chv_phy_pre_pll_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 ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
> enum pipe pipe = crtc->pipe;
> unsigned int lane_mask =
> intel_dp_unused_lane_mask(crtc_state->lane_count);
> @@ -941,9 +940,8 @@ void chv_phy_pre_encoder_enable(struct intel_encoder *encoder,
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> 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 ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
> int data, i, stagger;
> u32 val;
>
> @@ -1030,8 +1028,8 @@ void chv_phy_post_pll_disable(struct intel_encoder *encoder,
> const struct intel_crtc_state *old_crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + enum dpio_phy phy = vlv_dig_port_to_phy(enc_to_dig_port(encoder));
> enum pipe pipe = to_intel_crtc(old_crtc_state->uapi.crtc)->pipe;
> - enum dpio_phy phy = vlv_pipe_to_phy(pipe);
> u32 val;
>
> vlv_dpio_get(dev_priv);
> @@ -1068,9 +1066,8 @@ void vlv_set_phy_signal_level(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> enum dpio_channel ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
>
> vlv_dpio_get(dev_priv);
>
> @@ -1095,9 +1092,8 @@ void vlv_phy_pre_pll_enable(struct intel_encoder *encoder,
> {
> struct intel_digital_port *dig_port = enc_to_dig_port(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 ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
>
> /* Program Tx lane resets to default */
> vlv_dpio_get(dev_priv);
> @@ -1127,8 +1123,8 @@ 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 ch = vlv_dig_port_to_channel(dig_port);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
> enum pipe pipe = crtc->pipe;
> - enum dpio_phy phy = vlv_pipe_to_phy(pipe);
> u32 val;
>
> vlv_dpio_get(dev_priv);
> @@ -1154,9 +1150,8 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder,
> {
> struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> enum dpio_channel ch = vlv_dig_port_to_channel(dig_port);
> - enum dpio_phy phy = vlv_pipe_to_phy(crtc->pipe);
> + enum dpio_phy phy = vlv_dig_port_to_phy(dig_port);
>
> vlv_dpio_get(dev_priv);
> vlv_dpio_write(dev_priv, phy, VLV_PCS_DW0(ch), 0x00000000);
> diff --git a/drivers/gpu/drm/i915/vlv_sideband.c b/drivers/gpu/drm/i915/vlv_sideband.c
> index ffa195560d0d..68291412f4cb 100644
> --- a/drivers/gpu/drm/i915/vlv_sideband.c
> +++ b/drivers/gpu/drm/i915/vlv_sideband.c
> @@ -9,7 +9,6 @@
> #include "vlv_sideband.h"
>
> #include "display/intel_dpio_phy.h"
> -#include "display/intel_display_types.h"
I guess this should be done in some other patch?
>
> /*
> * IOSF sideband, see VLV2_SidebandMsg_HAS.docx and
--
Jani Nikula, Intel
