From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 10/10] drm/i915: Clean up DP code local variables and calling conventions
Date: Thu, 9 Nov 2017 17:27:13 +0200 [thread overview]
Message-ID: <20171109152713.GS10981@intel.com> (raw)
In-Reply-To: <1510197699.8287.35.camel@dk-H97M-D3H>
On Thu, Nov 09, 2017 at 03:01:11AM +0000, Pandiyan, Dhinakaran wrote:
>
>
> nits below.
>
>
> On Tue, 2017-10-31 at 22:51 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Eliminate a ton of pointless 'dev' variables in the DP code, and pass
> > around 'dev_priv' instead of 'dev'.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 151 ++++++++++++++--------------------------
> > 1 file changed, 54 insertions(+), 97 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index f875ba78c435..30ac187b2b97 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -438,10 +438,7 @@ intel_dp_pps_init(struct intel_dp *intel_dp);
> >
> > static void pps_lock(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct intel_encoder *encoder = &intel_dig_port->base;
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > /*
> > * See vlv_power_sequencer_reset() why we need
> > @@ -454,10 +451,7 @@ static void pps_lock(struct intel_dp *intel_dp)
> >
> > static void pps_unlock(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct intel_encoder *encoder = &intel_dig_port->base;
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > mutex_unlock(&dev_priv->pps_mutex);
> >
> > @@ -467,8 +461,8 @@ static void pps_unlock(struct intel_dp *intel_dp)
> > static void
> > vlv_power_sequencer_kick(struct intel_dp *intel_dp)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
> > enum pipe pipe = intel_dp->pps_pipe;
> > bool pll_enabled, release_cl_override = false;
> > enum dpio_phy phy = DPIO_PHY(pipe);
> > @@ -733,7 +727,6 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
> >
> > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > {
> > - struct drm_device *dev = &dev_priv->drm;
> > struct intel_encoder *encoder;
> >
> > if (WARN_ON(!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
> > @@ -750,7 +743,7 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > * should use them always.
> > */
> >
> > - for_each_intel_encoder(dev, encoder) {
> > + for_each_intel_encoder(&dev_priv->drm, encoder) {
> > struct intel_dp *intel_dp;
> >
> > if (encoder->type != INTEL_OUTPUT_DP &&
> > @@ -832,8 +825,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> > {
> > struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
> > edp_notifier);
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
> > return 0;
> > @@ -863,8 +855,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> >
> > static bool edp_have_panel_power(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > @@ -877,8 +868,7 @@ static bool edp_have_panel_power(struct intel_dp *intel_dp)
> >
> > static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > @@ -892,8 +882,7 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
> > static void
> > intel_dp_check_edp(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > if (!intel_dp_is_edp(intel_dp))
> > return;
> > @@ -909,9 +898,7 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > static uint32_t
> > intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
> > uint32_t status;
> > bool done;
> > @@ -1478,8 +1465,7 @@ static void
> > intel_dp_set_clock(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config)
> > {
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > const struct dp_link_dpll *divisor = NULL;
> > int i, count = 0;
> >
> > @@ -1848,8 +1834,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> > static void intel_dp_prepare(struct intel_encoder *encoder,
> > const struct intel_crtc_state *pipe_config)
> > {
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > enum port port = dp_to_dig_port(intel_dp)->base.port;
> > struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> > @@ -1945,8 +1930,7 @@ static void wait_panel_status(struct intel_dp *intel_dp,
> > u32 mask,
> > u32 value)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> > @@ -2022,8 +2006,7 @@ static void edp_wait_backlight_off(struct intel_dp *intel_dp)
> >
> > static u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > u32 control;
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> > @@ -2044,9 +2027,8 @@ static u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
> > */
> > static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > u32 pp;
> > i915_reg_t pp_stat_reg, pp_ctrl_reg;
> > bool need_to_disable = !intel_dp->want_panel_vdd;
> > @@ -2116,8 +2098,7 @@ void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
> >
> > static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > u32 pp;
> > @@ -2203,8 +2184,7 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
> >
> > static void edp_panel_on(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > u32 pp;
> > i915_reg_t pp_ctrl_reg;
> >
> > @@ -2262,8 +2242,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
> >
> > static void edp_panel_off(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > u32 pp;
> > i915_reg_t pp_ctrl_reg;
> >
> > @@ -2311,9 +2290,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
> > /* Enable backlight in the panel power control. */
> > static void _intel_edp_backlight_on(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > u32 pp;
> > i915_reg_t pp_ctrl_reg;
> >
> > @@ -2356,8 +2333,7 @@ void intel_edp_backlight_on(const struct intel_crtc_state *crtc_state,
> > /* Disable backlight in the panel power control. */
> > static void _intel_edp_backlight_off(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > u32 pp;
> > i915_reg_t pp_ctrl_reg;
> >
> > @@ -2560,10 +2536,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> > static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> > enum pipe *pipe)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > enum port port = dp_to_dig_port(intel_dp)->base.port;
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > u32 tmp;
> > bool ret;
> >
> > @@ -2612,10 +2587,9 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> > static void intel_dp_get_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > u32 tmp, flags = 0;
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > enum port port = dp_to_dig_port(intel_dp)->base.port;
> > struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> >
> > @@ -2782,9 +2756,8 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > uint32_t *DP,
> > uint8_t dp_train_pat)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > enum port port = intel_dig_port->base.port;
> >
> > if (dp_train_pat & DP_TRAINING_PATTERN_MASK)
> > @@ -2868,8 +2841,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
> > static void intel_dp_enable_port(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *old_crtc_state)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > /* enable with pattern 1 (as per spec) */
> >
> > @@ -2893,9 +2865,8 @@ static void intel_enable_dp(struct intel_encoder *encoder,
> > const struct intel_crtc_state *pipe_config,
> > const struct drm_connector_state *conn_state)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > - struct drm_device *dev = encoder->base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> > uint32_t dp_reg = I915_READ(intel_dp->output_reg);
> > enum pipe pipe = crtc->pipe;
> > @@ -3519,10 +3490,9 @@ gen7_edp_signal_levels(uint8_t train_set)
> > void
> > intel_dp_set_signal_levels(struct intel_dp *intel_dp)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > enum port port = intel_dig_port->base.port;
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > uint32_t signal_levels, mask = 0;
> > uint8_t train_set = intel_dp->train_set[0];
> >
> > @@ -3577,9 +3547,8 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
> >
> > void intel_dp_set_idle_link_train(struct intel_dp *intel_dp)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > enum port port = intel_dig_port->base.port;
> > uint32_t val;
> >
> > @@ -4303,11 +4272,11 @@ intel_dp_retrain_link(struct intel_dp *intel_dp)
> > static void
> > intel_dp_check_link_status(struct intel_dp *intel_dp)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>
> You convert it back again to drm_device.
Actually where I want to go next is changing intel_dp_to_dev() into
intel_dp_to_i915(). So at that point things will not look so odd.
>
> > struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > u8 link_status[DP_LINK_STATUS_SIZE];
> >
> > - WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> > + WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
> >
> > if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > DRM_ERROR("Failed to get link status\n");
> > @@ -4353,8 +4322,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> > static bool
> > intel_dp_short_pulse(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>
> to_i915() is unnecessary, all cases of dev_priv usage are for
> dev_priv->drm.
similar reasoning here
>
>
> > u8 sink_irq_vector = 0;
> > u8 old_sink_count = intel_dp->sink_count;
> > bool ret;
> > @@ -4393,13 +4361,13 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
> > DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
> > }
> >
> > - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > + drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);
> > - drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > + drm_modeset_unlock(&dev_priv->drm.mode_config.connection_mutex);
> > if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> > DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
> > /* Send a Hotplug Uevent to userspace to start modeset */
> > - drm_kms_helper_hotplug_event(intel_encoder->base.dev);
> > + drm_kms_helper_hotplug_event(&dev_priv->drm);
> > }
> >
> > return true;
> > @@ -4463,8 +4431,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > static enum drm_connector_status
> > edp_detect(struct intel_dp *intel_dp)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > enum drm_connector_status status;
> >
> > status = intel_panel_detect(dev_priv);
> > @@ -4720,22 +4687,21 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > }
> >
> > static int
> > -intel_dp_long_pulse(struct intel_connector *intel_connector)
> > +intel_dp_long_pulse(struct intel_connector *connector)
> > {
> > - struct drm_connector *connector = &intel_connector->base;
> > - struct intel_dp *intel_dp = intel_attached_dp(connector);
> > - struct drm_device *dev = connector->dev;
> > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > + struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> > enum drm_connector_status status;
> > u8 sink_irq_vector = 0;
> >
> > - WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
> > + WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
> >
> > - intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
> > + intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >
> > /* Can't disconnect eDP, but you can close the lid... */
> > if (intel_dp_is_edp(intel_dp))
> > status = edp_detect(intel_dp);
> > - else if (intel_digital_port_connected(to_i915(dev),
> > + else if (intel_digital_port_connected(dev_priv,
> > dp_to_dig_port(intel_dp)))
> > status = intel_dp_detect_dpcd(intel_dp);
> > else
> > @@ -4806,7 +4772,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> > intel_dp->aux.i2c_defer_count = 0;
> >
> > intel_dp_set_edid(intel_dp);
> > - if (intel_dp_is_edp(intel_dp) || intel_connector->detect_edid)
> > + if (intel_dp_is_edp(intel_dp) || connector->detect_edid)
> > status = connector_status_connected;
> > intel_dp->detect_done = true;
> >
> > @@ -4829,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> > if (status != connector_status_connected && !intel_dp->is_mst)
> > intel_dp_unset_edid(intel_dp);
> >
> > - intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
> > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > return status;
> > }
> >
> > @@ -4996,9 +4962,7 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> >
> > static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> > {
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > @@ -5086,8 +5050,7 @@ enum irqreturn
> > intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > {
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > - struct drm_device *dev = intel_dig_port->base.base.dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > enum irqreturn ret = IRQ_NONE;
> >
> > if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> > @@ -5546,8 +5509,7 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,
> > void intel_edp_drrs_enable(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *crtc_state)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > if (!crtc_state->has_drrs) {
> > DRM_DEBUG_KMS("Panel doesn't support DRRS\n");
> > @@ -5582,8 +5544,7 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
> > void intel_edp_drrs_disable(struct intel_dp *intel_dp,
> > const struct intel_crtc_state *old_crtc_state)
> > {
> > - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> >
> > if (!old_crtc_state->has_drrs)
> > return;
> > @@ -5766,7 +5727,7 @@ void intel_edp_drrs_flush(struct drm_i915_private *dev_priv,
> >
> > /**
> > * intel_dp_drrs_init - Init basic DRRS work and mutex.
> > - * @intel_connector: eDP connector
> > + * @connector: eDP connector
> > * @fixed_mode: preferred mode of panel
> > *
> > * This function is called only once at driver load to initialize basic
> > @@ -5778,12 +5739,10 @@ void intel_edp_drrs_flush(struct drm_i915_private *dev_priv,
> > * from VBT setting).
> > */
> > static struct drm_display_mode *
> > -intel_dp_drrs_init(struct intel_connector *intel_connector,
> > - struct drm_display_mode *fixed_mode)
> > +intel_dp_drrs_init(struct intel_connector *connector,
> > + struct drm_display_mode *fixed_mode)
> > {
> > - struct drm_connector *connector = &intel_connector->base;
> > - struct drm_device *dev = connector->dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > struct drm_display_mode *downclock_mode = NULL;
> >
> > INIT_DELAYED_WORK(&dev_priv->drrs.work, intel_edp_drrs_downclock_work);
> > @@ -5799,8 +5758,8 @@ intel_dp_drrs_init(struct intel_connector *intel_connector,
> > return NULL;
> > }
> >
> > - downclock_mode = intel_find_panel_downclock
> > - (dev_priv, fixed_mode, connector);
> > + downclock_mode = intel_find_panel_downclock(dev_priv, fixed_mode,
> > + &connector->base);
> >
> > if (!downclock_mode) {
> > DRM_DEBUG_KMS("Downclock mode is not found. DRRS not supported\n");
> > @@ -5817,11 +5776,9 @@ intel_dp_drrs_init(struct intel_connector *intel_connector,
> > static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > struct intel_connector *intel_connector)
> > {
> > - struct drm_connector *connector = &intel_connector->base;
> > - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > - struct drm_device *dev = intel_encoder->base.dev;
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> You have retained dev here but
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_connector *connector = &intel_connector->base;
> > struct drm_display_mode *fixed_mode = NULL;
> > struct drm_display_mode *alt_fixed_mode = NULL;
> > struct drm_display_mode *downclock_mode = NULL;
> > @@ -5839,7 +5796,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > * eDP and LVDS bail out early in this case to prevent interfering
> > * with an already powered-on LVDS power sequencer.
> > */
> > - if (intel_get_lvds_encoder(dev)) {
> > + if (intel_get_lvds_encoder(&dev_priv->drm)) {
>
> but dereferenced dev_priv->drm here.
A bit redundant indeed. Although we should actually change
intel_get_lvds_encoder() to take dev_priv instead of dev, at which point
the issue becomes moot.
> > WARN_ON(!(HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv)));
> > DRM_INFO("LVDS was detected, not registering eDP\n");
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-11-09 15:27 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-31 20:51 [PATCH 00/10] drm/i915: Nuke dig_port->port and assorted cleanups Ville Syrjala
2017-10-31 20:51 ` [PATCH 01/10] drm/i915: Eliminate some encoder->crtc usage from DP code Ville Syrjala
2017-11-09 1:35 ` Pandiyan, Dhinakaran
2017-11-09 14:36 ` Ville Syrjälä
2017-10-31 20:51 ` [PATCH 02/10] drm/i915: Eliminate some encoder->crtc usage from DSI code Ville Syrjala
2017-11-09 1:36 ` Pandiyan, Dhinakaran
2017-11-09 14:43 ` Ville Syrjälä
2017-10-31 20:51 ` [PATCH 03/10] drm/i915: Eliminate some encoder->crtc usage from SDVO code Ville Syrjala
2017-10-31 20:51 ` [PATCH 04/10] drm/i915: Eliminate some encoder->crtc usage from TV code Ville Syrjala
2017-10-31 20:51 ` [PATCH 05/10] drm/i915: Pass crtc state to DPIO PHY functions Ville Syrjala
2017-10-31 20:51 ` [PATCH 06/10] drm/i915: Eliminate crtc->config usage from CRT code Ville Syrjala
2017-10-31 20:51 ` [PATCH 07/10] drm/i915: Replace dig_port->port with encoder port for BXT DPLL selection Ville Syrjala
2017-10-31 20:51 ` [PATCH 08/10] drm/i915: Nuke intel_digital_port->port Ville Syrjala
2017-11-09 1:37 ` Pandiyan, Dhinakaran
2017-11-09 13:50 ` Ville Syrjälä
2017-11-09 15:24 ` [PATCH v2 " Ville Syrjala
2017-10-31 20:51 ` [PATCH 09/10] drm/i915: Clean up PPS code calling conventions Ville Syrjala
2017-10-31 20:51 ` [PATCH 10/10] drm/i915: Clean up DP code local variables and " Ville Syrjala
2017-11-09 3:01 ` Pandiyan, Dhinakaran
2017-11-09 15:27 ` Ville Syrjälä [this message]
2017-11-09 15:27 ` [PATCH v2 " Ville Syrjala
2017-10-31 22:31 ` ✗ Fi.CI.BAT: warning for drm/i915: Nuke dig_port->port and assorted cleanups Patchwork
2017-11-01 9:55 ` [PATCH 00/10] " Jani Nikula
2017-11-09 3:08 ` Pandiyan, Dhinakaran
2017-11-09 18:26 ` Ville Syrjälä
2017-11-09 16:01 ` ✓ Fi.CI.BAT: success for drm/i915: Nuke dig_port->port and assorted cleanups (rev3) Patchwork
2017-11-09 16:59 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171109152713.GS10981@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=dhinakaran.pandiyan@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).