| Message ID | 1389993418-2133-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 17, 2014 at 01:16:56PM -0800, Jesse Barnes wrote: > In DDI configs, we need to get the encoder to CRTC mapping early on so > we can read out and calculate the clock state correctly, as it depends > on the port. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++--------- > 1 file changed, 44 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 74137d5..92f46ad 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev) > "crtc's computed enabled state doesn't match tracked enabled state " > "(expected %i, found %i)\n", enabled, crtc->base.enabled); > > + > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + /* Get encoder->crtc mapping */ > + struct intel_crtc *tmp_crtc; > + enum pipe pipe; > + if (encoder->get_hw_state(encoder, &pipe)) { > + tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + if (tmp_crtc == crtc) { > + encoder->base.crtc = &tmp_crtc->base; > + encoder->get_config(encoder, > + &pipe_config); > + } > + } > + } > + > active = dev_priv->display.get_pipe_config(crtc, > &pipe_config); > NAK. This will break clock readout for every other platform. For most things we read out the .port_clock from the DPLL in .get_pipe_config() (eDP port A being the excption where intel_dp_get_config() also fills out .port_clock), and then the encoder .get_config() massages .port_clock appropriately to figure out what .crtc_clock was supposed to be. Maybe I'm missing something, but can't you just follow the eDP port A route and simply stick your intel_ddi_clock_get() into intel_ddi_get_config(), so that intel_ddi_get_config() is always responsible for filling out both .port_clock and .crtc_clock? > @@ -11109,9 +11125,26 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > int i; > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, > - base.head) { > + base.head) > memset(&crtc->config, 0, sizeof(crtc->config)); > > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + pipe = 0; > + > + if (encoder->get_hw_state(encoder, &pipe)) { > + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + encoder->base.crtc = &crtc->base; > + drm_mode_debug_printmodeline(&crtc->config.adjusted_mode); > + } else { > + encoder->base.crtc = NULL; > + } > + > + encoder->connectors_active = false; > + } > + > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, > + base.head) { > crtc->active = dev_priv->display.get_pipe_config(crtc, > &crtc->config); > > @@ -11145,22 +11178,18 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > list_for_each_entry(encoder, &dev->mode_config.encoder_list, > base.head) { > - pipe = 0; > - > - if (encoder->get_hw_state(encoder, &pipe)) { > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > - encoder->base.crtc = &crtc->base; > + if (encoder->base.crtc) { > + crtc = to_intel_crtc(encoder->base.crtc); > encoder->get_config(encoder, &crtc->config); > - } else { > - encoder->base.crtc = NULL; > - } > > - encoder->connectors_active = false; > - DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n", > - encoder->base.base.id, > - drm_get_encoder_name(&encoder->base), > - encoder->base.crtc ? "enabled" : "disabled", > - pipe_name(pipe)); > + DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c, dp: %s\n", > + encoder->base.base.id, > + drm_get_encoder_name(&encoder->base), > + encoder->base.crtc ? "enabled" : "disabled", > + pipe_name(crtc->pipe), > + crtc->config.has_dp_encoder ? "yes" : "no"); > + drm_mode_debug_printmodeline(&crtc->config.adjusted_mode); > + } > } > > list_for_each_entry(connector, &dev->mode_config.connector_list, > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, 18 Jan 2014 16:01:06 +0200 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Jan 17, 2014 at 01:16:56PM -0800, Jesse Barnes wrote: > > In DDI configs, we need to get the encoder to CRTC mapping early on so > > we can read out and calculate the clock state correctly, as it depends > > on the port. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++--------- > > 1 file changed, 44 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 74137d5..92f46ad 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev) > > "crtc's computed enabled state doesn't match tracked enabled state " > > "(expected %i, found %i)\n", enabled, crtc->base.enabled); > > > > + > > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > > + base.head) { > > + /* Get encoder->crtc mapping */ > > + struct intel_crtc *tmp_crtc; > > + enum pipe pipe; > > + if (encoder->get_hw_state(encoder, &pipe)) { > > + tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > + if (tmp_crtc == crtc) { > > + encoder->base.crtc = &tmp_crtc->base; > > + encoder->get_config(encoder, > > + &pipe_config); > > + } > > + } > > + } > > + > > active = dev_priv->display.get_pipe_config(crtc, > > &pipe_config); > > > > NAK. This will break clock readout for every other platform. For most > things we read out the .port_clock from the DPLL in .get_pipe_config() > (eDP port A being the excption where intel_dp_get_config() also fills > out .port_clock), and then the encoder .get_config() massages .port_clock > appropriately to figure out what .crtc_clock was supposed to be. > > Maybe I'm missing something, but can't you just follow the eDP port A > route and simply stick your intel_ddi_clock_get() into > intel_ddi_get_config(), so that intel_ddi_get_config() is always > responsible for filling out both .port_clock and .crtc_clock? > Yeah I knew this would cause trouble. I might be able to get the clock later, but I just need to make sure the CRTC to encoder mapping is set up when I need it.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 74137d5..92f46ad 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9476,6 +9476,22 @@ check_crtc_state(struct drm_device *dev) "crtc's computed enabled state doesn't match tracked enabled state " "(expected %i, found %i)\n", enabled, crtc->base.enabled); + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, + base.head) { + /* Get encoder->crtc mapping */ + struct intel_crtc *tmp_crtc; + enum pipe pipe; + if (encoder->get_hw_state(encoder, &pipe)) { + tmp_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); + if (tmp_crtc == crtc) { + encoder->base.crtc = &tmp_crtc->base; + encoder->get_config(encoder, + &pipe_config); + } + } + } + active = dev_priv->display.get_pipe_config(crtc, &pipe_config); @@ -11109,9 +11125,26 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) int i; list_for_each_entry(crtc, &dev->mode_config.crtc_list, - base.head) { + base.head) memset(&crtc->config, 0, sizeof(crtc->config)); + list_for_each_entry(encoder, &dev->mode_config.encoder_list, + base.head) { + pipe = 0; + + if (encoder->get_hw_state(encoder, &pipe)) { + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); + encoder->base.crtc = &crtc->base; + drm_mode_debug_printmodeline(&crtc->config.adjusted_mode); + } else { + encoder->base.crtc = NULL; + } + + encoder->connectors_active = false; + } + + list_for_each_entry(crtc, &dev->mode_config.crtc_list, + base.head) { crtc->active = dev_priv->display.get_pipe_config(crtc, &crtc->config); @@ -11145,22 +11178,18 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) { - pipe = 0; - - if (encoder->get_hw_state(encoder, &pipe)) { - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); - encoder->base.crtc = &crtc->base; + if (encoder->base.crtc) { + crtc = to_intel_crtc(encoder->base.crtc); encoder->get_config(encoder, &crtc->config); - } else { - encoder->base.crtc = NULL; - } - encoder->connectors_active = false; - DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n", - encoder->base.base.id, - drm_get_encoder_name(&encoder->base), - encoder->base.crtc ? "enabled" : "disabled", - pipe_name(pipe)); + DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c, dp: %s\n", + encoder->base.base.id, + drm_get_encoder_name(&encoder->base), + encoder->base.crtc ? "enabled" : "disabled", + pipe_name(crtc->pipe), + crtc->config.has_dp_encoder ? "yes" : "no"); + drm_mode_debug_printmodeline(&crtc->config.adjusted_mode); + } } list_for_each_entry(connector, &dev->mode_config.connector_list,
In DDI configs, we need to get the encoder to CRTC mapping early on so we can read out and calculate the clock state correctly, as it depends on the port. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_display.c | 59 +++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 15 deletions(-)