On 2019-12-04 at 20:05:45 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> It's easy to confuse the drm_connector->encoder (legacy state
> adjusted during modeset) and intel_connector->encoder (the statically
> (sans. MST) attached encoder of the connector). For the latter
> let's use intel_attached_encoder() consistently.
>
> @@
> identifier F !~ "^intel_attached_encoder$";
> struct intel_connector *C;
> expression E;
> @@
> F(...)
> {
> <...
> (
> C->encoder = E
> |
> - C->encoder
> + intel_attached_encoder(C)
You mean intel_attached_encoder(&C->base)
Need to pass the drm_connector *
So this is for readability!? This adds the line length beyond 80char
or to reduce that a extra local variable. I leave that to you.
> )
> ...>
> }
>
> @@
> identifier F !~ "^intel_attached_encoder$";
> struct drm_connector *C;
> expression E;
> @@
> F(...)
> {
> <...
> (
> to_intel_connector(C)->encoder = E
> |
> - to_intel_connector(C)->encoder
> + intel_attached_encoder(to_intel_connector(C))
> )
> ...>
> }
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_connector.c | 2 +-
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
> .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 12 ++++++------
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++--
> .../gpu/drm/i915/display/intel_dsi_dcs_backlight.c | 4 ++--
> drivers/gpu/drm/i915/display/intel_hdcp.c | 8 ++++----
> drivers/gpu/drm/i915/display/intel_hotplug.c | 10 +++++-----
> drivers/gpu/drm/i915/display/vlv_dsi.c | 2 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +++---
> 10 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 1133c4e97bb4..54891a4ed2f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -153,7 +153,7 @@ void intel_connector_attach_encoder(struct intel_connector *connector,
> bool intel_connector_get_hw_state(struct intel_connector *connector)
> {
> enum pipe pipe = 0;
> - struct intel_encoder *encoder = connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(connector);
intel_attached_encoder takes drm_connector * instead of intel_connector.
Everywhere we need to change this.
>
> return encoder->get_hw_state(encoder, &pipe);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 52d187db320f..3e46017150b4 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2003,7 +2003,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
> {
> struct drm_device *dev = intel_connector->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_encoder *encoder = intel_connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(intel_connector);
As you have done in previous series, could we rename this intel_connector
as connector
> int type = intel_connector->base.connector_type;
> enum port port = encoder->port;
> enum transcoder cpu_transcoder;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 45568a7c6579..df1b80387106 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7279,7 +7279,7 @@ static void intel_connector_verify_state(struct intel_crtc_state *crtc_state,
> connector->base.name);
>
> if (connector->get_hw_state(connector)) {
> - struct intel_encoder *encoder = connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(connector);
>
> I915_STATE_WARN(!crtc_state,
> "connector enabled without attached crtc\n");
> @@ -17529,7 +17529,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> connector->base.dpms = DRM_MODE_DPMS_ON;
>
> - encoder = connector->encoder;
> + encoder = intel_attached_encoder(connector);
> connector->base.encoder = &encoder->base;
>
> crtc = to_intel_crtc(encoder->base.crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index 7c653f8c307f..771e22a0b2a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -57,7 +57,7 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable)
> */
> static u32 intel_dp_aux_get_backlight(struct intel_connector *connector)
> {
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
> u8 read_val[2] = { 0x0 };
> u16 level = 0;
>
> @@ -82,7 +82,7 @@ static void
> intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 level)
> {
> struct intel_connector *connector = to_intel_connector(conn_state->connector);
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
> u8 vals[2] = { 0x0 };
>
> vals[0] = level;
> @@ -110,7 +110,7 @@ intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 lev
> static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
> int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> u8 pn, pn_min, pn_max;
>
> @@ -178,7 +178,7 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
> const struct drm_connector_state *conn_state)
> {
> struct intel_connector *connector = to_intel_connector(conn_state->connector);
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
> u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> @@ -229,7 +229,7 @@ static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old
> static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
> enum pipe pipe)
> {
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
> struct intel_panel *panel = &connector->panel;
>
> if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
> @@ -248,7 +248,7 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
> static bool
> intel_dp_aux_display_control_capable(struct intel_connector *connector)
> {
> - struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
> + struct intel_dp *intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
>
> /* Check the eDP Display control capabilities registers to determine if
> * the panel can support backlight control over the aux channel
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index af7f6d670e07..1a7d69843c12 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -503,9 +503,9 @@ static const struct drm_encoder_funcs intel_dp_mst_enc_funcs = {
>
> static bool intel_dp_mst_get_hw_state(struct intel_connector *connector)
> {
> - if (connector->encoder && connector->base.state->crtc) {
> + if (intel_attached_encoder(connector) && connector->base.state->crtc) {
> enum pipe pipe;
> - if (!connector->encoder->get_hw_state(connector->encoder, &pipe))
> + if (!intel_attached_encoder(connector)->get_hw_state(intel_attached_encoder(connector), &pipe))
Do we prefer this way or with local var for attached encoder restricting
line to 80char?
> return false;
> return true;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> index c87838843d0b..ac3eff26df12 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c
> @@ -45,7 +45,7 @@
>
> static u32 dcs_get_backlight(struct intel_connector *connector)
> {
> - struct intel_encoder *encoder = connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(connector);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> struct mipi_dsi_device *dsi_device;
> u8 data = 0;
> @@ -160,7 +160,7 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector)
> {
> struct drm_device *dev = intel_connector->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_encoder *encoder = intel_connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(intel_connector);
s/intel_connector/connector ?
> struct intel_panel *panel = &intel_connector->panel;
>
> if (dev_priv->vbt.backlight.type != INTEL_BACKLIGHT_DSI_DCS)
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0fdbd39f6641..2859230671ae 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1527,7 +1527,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector)
> struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = &connector->hdcp;
> - enum port port = connector->encoder->port;
> + enum port port = intel_attached_encoder(connector)->port;
> enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
> int ret;
>
> @@ -1565,7 +1565,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector)
> struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = &connector->hdcp;
> - enum port port = connector->encoder->port;
> + enum port port = intel_attached_encoder(connector)->port;
> enum transcoder cpu_transcoder = hdcp->cpu_transcoder;
> int ret;
>
> @@ -1676,7 +1676,7 @@ static int intel_hdcp2_check_link(struct intel_connector *connector)
> struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = &connector->hdcp;
> - enum port port = connector->encoder->port;
> + enum port port = intel_attached_encoder(connector)->port;
> enum transcoder cpu_transcoder;
> int ret = 0;
>
> @@ -1830,7 +1830,7 @@ static inline int initialize_hdcp_port_data(struct intel_connector *connector,
>
> if (INTEL_GEN(dev_priv) < 12)
> data->fw_ddi =
> - intel_get_mei_fw_ddi_index(connector->encoder->port);
> + intel_get_mei_fw_ddi_index(intel_attached_encoder(connector)->port);
could we wrap at 80char with var encoder?
> else
> /*
> * As per ME FW API expectation, for GEN 12+, fw_ddi is filled
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 99d3a3c7989e..c3a48af35997 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -200,7 +200,7 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv)
> continue;
>
> intel_connector = to_intel_connector(connector);
> - intel_encoder = intel_connector->encoder;
> + intel_encoder = intel_attached_encoder(intel_connector);
> if (!intel_encoder)
> continue;
>
> @@ -255,7 +255,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>
> /* Don't check MST ports, they don't have pins */
> if (!intel_connector->mst_port &&
> - intel_connector->encoder->hpd_pin == pin) {
> + intel_attached_encoder(intel_connector)->hpd_pin == pin) {
> if (connector->polled != intel_connector->polled)
> DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> connector->name);
> @@ -389,9 +389,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
> u32 hpd_bit;
>
> intel_connector = to_intel_connector(connector);
> - if (!intel_connector->encoder)
> + if (!intel_attached_encoder(intel_connector))
> continue;
> - intel_encoder = intel_connector->encoder;
> + intel_encoder = intel_attached_encoder(intel_connector);
> hpd_bit = BIT(intel_encoder->hpd_pin);
> if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
> DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
> @@ -621,7 +621,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
> continue;
>
> if (!connector->polled && I915_HAS_HOTPLUG(dev_priv) &&
> - intel_connector->encoder->hpd_pin > HPD_NONE) {
> + intel_attached_encoder(intel_connector)->hpd_pin > HPD_NONE) {
> connector->polled = enabled ?
> DRM_CONNECTOR_POLL_CONNECT |
> DRM_CONNECTOR_POLL_DISCONNECT :
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 0c19064e42e0..535b332b067d 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1567,7 +1567,7 @@ static enum drm_panel_orientation
> vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> - struct intel_encoder *encoder = connector->encoder;
> + struct intel_encoder *encoder = intel_attached_encoder(connector);
> enum intel_display_power_domain power_domain;
> enum drm_panel_orientation orientation;
> struct intel_plane *plane;
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3bc8d5c0e88a..d8c2fa2672b5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2431,7 +2431,7 @@ static void intel_hdcp_info(struct seq_file *m,
> static void intel_dp_info(struct seq_file *m,
> struct intel_connector *intel_connector)
> {
> - struct intel_encoder *intel_encoder = intel_connector->encoder;
> + struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector);
> struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
>
> seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]);
> @@ -2450,7 +2450,7 @@ static void intel_dp_info(struct seq_file *m,
> static void intel_dp_mst_info(struct seq_file *m,
> struct intel_connector *intel_connector)
> {
> - struct intel_encoder *intel_encoder = intel_connector->encoder;
> + struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector);
s/intel_connector/connector ?
> struct intel_dp_mst_encoder *intel_mst =
> enc_to_mst(intel_encoder);
> struct intel_digital_port *intel_dig_port = intel_mst->primary;
> @@ -2464,7 +2464,7 @@ static void intel_dp_mst_info(struct seq_file *m,
> static void intel_hdmi_info(struct seq_file *m,
> struct intel_connector *intel_connector)
> {
> - struct intel_encoder *intel_encoder = intel_connector->encoder;
> + struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector);
s/intel_connector/connector ?
-Ram
> struct intel_dp_mst_encoder *intel_mst =
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(intel_encoder);
>
> seq_printf(m, "\taudio support: %s\n", yesno(intel_hdmi->has_audio));
> --
> 2.23.0
>
> _______________________________________________
> 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
