> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > Ville Syrjälä > Sent: Thursday, October 12, 2017 9:59 PM > To: Nikula, Jani <jani.nikula@xxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 3/4] drm/i915: push crtc compute clock to > encoder compute config on DDI > > On Thu, Oct 12, 2017 at 07:05:32PM +0300, Jani Nikula wrote: > > With this, we can get rid of the encoder specific stuff that we had in > > haswell_crtc_compute_clock(), which we throw away entirely. This also > > paves the way for potential further untangling of encoder specific > > checks in the depths of intel_get_shared_dpll() that plague the DDI > > platforms. > > > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_crt.c | 7 +++++++ > > drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++ > > drivers/gpu/drm/i915/intel_display.c | 21 --------------------- > > 3 files changed, 14 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_crt.c > > b/drivers/gpu/drm/i915/intel_crt.c > > index 437339f5d098..67f771f24608 100644 > > --- a/drivers/gpu/drm/i915/intel_crt.c > > +++ b/drivers/gpu/drm/i915/intel_crt.c > > @@ -361,6 +361,7 @@ static bool hsw_crt_compute_config(struct > intel_encoder *encoder, > > struct drm_connector_state *conn_state) { > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); > > > > pipe_config->has_pch_encoder = true; > > > > @@ -377,6 +378,12 @@ static bool hsw_crt_compute_config(struct > intel_encoder *encoder, > > /* FDI must always be 2.7 GHz */ > > pipe_config->port_clock = 135000 * 2; > > > > + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) { > > + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n", > > + pipe_name(crtc->pipe)); > > + return false; > > + } > > I'm not convinced we can do this. intel_modeset_clear_plls() hasn't even > been called yet. Agree. How about calling intel_modeset_clear_plls() before encoder->compute_config() Inside intel_modeset_pipe_config() instead of intel_modeset_checks()?? Looked at modeset code briefly, couldn’t find any user of DPLL between intel_modeset_pipe_config() and intel_modeset_checks(). Please correct if wrong. Regards, Madhav > > > + > > return true; > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index b307b6fe1ce3..9819e51fa160 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2590,6 +2590,7 @@ static bool intel_ddi_compute_config(struct > intel_encoder *encoder, > > struct drm_connector_state *conn_state) > { > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); > > int type = encoder->type; > > int port = intel_ddi_get_encoder_port(encoder); > > int ret; > > @@ -2609,6 +2610,12 @@ static bool intel_ddi_compute_config(struct > intel_encoder *encoder, > > bxt_ddi_phy_calc_lane_lat_optim_mask(encoder, > > pipe_config- > >lane_count); > > > > + if (!intel_get_shared_dpll(crtc, pipe_config, encoder)) { > > + DRM_DEBUG_KMS("failed to find PLL for pipe %c\n", > > + pipe_name(crtc->pipe)); > > + return false; > > + } > > + > > return ret; > > > > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 2faa938f57a3..6ed299670f27 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9011,23 +9011,6 @@ void hsw_disable_pc8(struct drm_i915_private > *dev_priv) > > } > > } > > > > -static int haswell_crtc_compute_clock(struct intel_crtc *crtc, > > - struct intel_crtc_state *crtc_state) > > -{ > > - if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) { > > - struct intel_encoder *encoder = > > - intel_ddi_get_crtc_new_encoder(crtc_state); > > - > > - if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) { > > - DRM_DEBUG_DRIVER("failed to find PLL for pipe > %c\n", > > - pipe_name(crtc->pipe)); > > - return -EINVAL; > > - } > > - } > > - > > - return 0; > > -} > > - > > static void cannonlake_get_ddi_pll(struct drm_i915_private *dev_priv, > > enum port port, > > struct intel_crtc_state *pipe_config) @@ - > 14141,16 +14124,12 > > @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv) > > dev_priv->display.get_pipe_config = > haswell_get_pipe_config; > > dev_priv->display.get_initial_plane_config = > > skylake_get_initial_plane_config; > > - dev_priv->display.crtc_compute_clock = > > - haswell_crtc_compute_clock; > > dev_priv->display.crtc_enable = haswell_crtc_enable; > > dev_priv->display.crtc_disable = haswell_crtc_disable; > > } else if (HAS_DDI(dev_priv)) { > > dev_priv->display.get_pipe_config = > haswell_get_pipe_config; > > dev_priv->display.get_initial_plane_config = > > ironlake_get_initial_plane_config; > > - dev_priv->display.crtc_compute_clock = > > - haswell_crtc_compute_clock; > > dev_priv->display.crtc_enable = haswell_crtc_enable; > > dev_priv->display.crtc_disable = haswell_crtc_disable; > > } else if (HAS_PCH_SPLIT(dev_priv)) { > > -- > > 2.11.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > 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
