> -----Original Message----- > From: Nikula, Jani > Sent: Monday, July 2, 2018 3:27 PM > To: Chauhan, Madhav <madhav.chauhan@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Zanoni, Paulo R <paulo.r.zanoni@xxxxxxxxx>; Shankar, Uma > <uma.shankar@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; > Chauhan, Madhav <madhav.chauhan@xxxxxxxxx> > Subject: Re: [PATCH 04/20] drm/i915/icl: Enable DSI IO power > > On Fri, 15 Jun 2018, Madhav Chauhan <madhav.chauhan@xxxxxxxxx> wrote: > > This patch configures mode of operation for DSI and enable DDI IO > > power by configuring power well. > > > > Signed-off-by: Madhav Chauhan <madhav.chauhan@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_dsi_new.c | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi_new.c > > b/drivers/gpu/drm/i915/intel_dsi_new.c > > index 0d325ca..5ec4016 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi_new.c > > +++ b/drivers/gpu/drm/i915/intel_dsi_new.c > > @@ -55,11 +55,33 @@ static void gen11_dsi_program_esc_clk_div(struct > intel_encoder *encoder) > > } > > } > > > > +static void gen11_dsi_enable_io_power(struct intel_encoder *encoder) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > > + enum port port; > > + u32 tmp; > > + > > + for_each_dsi_port(port, intel_dsi->ports) { > > + tmp = I915_READ(ICL_DSI_IO_MODECTL(port)); > > + tmp |= COMBO_PHY_MODE_DSI; > > + I915_WRITE(ICL_DSI_IO_MODECTL(port), tmp); > > + } > > + > > + intel_display_power_get(dev_priv, > POWER_DOMAIN_PORT_DDI_A_IO); > > + > > + if (intel_dsi->dual_link) > > + intel_display_power_get(dev_priv, > POWER_DOMAIN_PORT_DDI_B_IO); > > Per bspec, "DSI0 uses DDI A; DSI1 uses DDI B" so shouldn't this be a > for_each_dsi_port() too, with the appropriate mapping to power domain? Do you mean something like this: enum intel_display_power_domain power_domain; for_each_dsi_port() { if (port ==PORT_A) power_domain = POWER_DOMAIN_PORT_DDI_A_IO; else if( port == PORT_B) power_domain = POWER_DOMAIN_PORT_DDI_A_IO; intel_display_power_get(dev_priv, power_domain); } > > > +} > > + > > static void __attribute__((unused)) gen11_dsi_pre_enable( > > struct intel_encoder *encoder, > > const struct intel_crtc_state *pipe_config, > > const struct drm_connector_state > *conn_state) > > Please don't end lines in (. You can add a newline before the function > name: > > static void __attribute__((unused)) > gen11_dsi_pre_enable(struct intel_encoder *encoder, > const struct intel_crtc_state *pipe_config, > const struct drm_connector_state *conn_state) > Got it. Thanks!! Regards, Madhav > > { > > + /* step2: enable IO power */ > > + gen11_dsi_enable_io_power(encoder); > > + > > /* step3: enable DSI PLL */ > > gen11_dsi_program_esc_clk_div(encoder); > > } > > -- > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx