* [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes()
@ 2015-03-11 16:52 ville.syrjala
2015-03-11 16:52 ` [PATCH 1/2] drm/i915: Rewrite some some of the FDI lane checks ville.syrjala
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: ville.syrjala @ 2015-03-11 16:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
So these on top of Ander's latest FDI bifurcation patch [1] should hopefully
result in something sane. Didn't test them though, but assuming Ander
has that test somewhere maybe some of the problematic cases I identified [2]
could be checked with these.
[1] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061802.html
[2] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061806.html
Ville Syrjälä (2):
drm/i915: Rewrite some some of the FDI lane checks
drm/i915: Rewrite IVB FDI bifurcation conflict checks
drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------------
1 file changed, 20 insertions(+), 20 deletions(-)
--
2.0.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] drm/i915: Rewrite some some of the FDI lane checks 2015-03-11 16:52 [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() ville.syrjala @ 2015-03-11 16:52 ` ville.syrjala 2015-03-11 16:52 ` [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks ville.syrjala 2015-03-12 7:28 ` [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() Ander Conselvan De Oliveira 2 siblings, 0 replies; 6+ messages in thread From: ville.syrjala @ 2015-03-11 16:52 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira From: Ville Syrjälä <ville.syrjala@linux.intel.com> The logic in the FDI lane checks is very hard for my poor brain to grasp. Rewrite it in a more straightforward way. Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c361af6..242a8a7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5589,14 +5589,13 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, } return true; case PIPE_C: - if (!pipe_has_enabled_pch(pipe_B_crtc) || - pipe_B_crtc->config->fdi_lanes <= 2) { - if (pipe_config->fdi_lanes > 2) { - DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n", - pipe_name(pipe), pipe_config->fdi_lanes); - return false; - } - } else { + if (pipe_config->fdi_lanes > 2) { + DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n", + pipe_name(pipe), pipe_config->fdi_lanes); + return false; + } + if (pipe_has_enabled_pch(pipe_B_crtc) && + pipe_B_crtc->config->fdi_lanes > 2) { DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n"); return false; } -- 2.0.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks 2015-03-11 16:52 [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() ville.syrjala 2015-03-11 16:52 ` [PATCH 1/2] drm/i915: Rewrite some some of the FDI lane checks ville.syrjala @ 2015-03-11 16:52 ` ville.syrjala 2015-03-12 7:35 ` Ander Conselvan De Oliveira 2015-03-12 7:28 ` [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() Ander Conselvan De Oliveira 2 siblings, 1 reply; 6+ messages in thread From: ville.syrjala @ 2015-03-11 16:52 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira From: Ville Syrjälä <ville.syrjala@linux.intel.com> Ignore the current state of the pipe and just check crtc_state->enable and the number of FDI lanes required. This means we don't accidentally mistake the FDI lanes as being available of one of the pipes just happens to be disabled at the time of the check. Also we no longer consider pipe C to require FDI lanes when it's driving the eDP transcoder. We also take the opportunity to make the code a bit nicer looking by hiding the ugly bits in the new pipe_required_fdi_lanes() function. Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 242a8a7..72e9816 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3152,12 +3152,6 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) FDI_FE_ERRC_ENABLE); } -static bool pipe_has_enabled_pch(struct intel_crtc *crtc) -{ - return crtc->base.state->enable && crtc->active && - crtc->config->has_pch_encoder; -} - /* The FDI link training functions for ILK/Ibexpeak. */ static void ironlake_fdi_link_train(struct drm_crtc *crtc) { @@ -5548,13 +5542,21 @@ bool intel_connector_get_hw_state(struct intel_connector *connector) return encoder->get_hw_state(encoder, &pipe); } +static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe) +{ + struct intel_crtc *crtc = + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe)); + + if (crtc->base.state->enable && + crtc->config->has_pch_encoder) + return crtc->config->fdi_lanes; + + return 0; +} + static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, struct intel_crtc_state *pipe_config) { - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *pipe_B_crtc = - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]); - DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n", pipe_name(pipe), pipe_config->fdi_lanes); if (pipe_config->fdi_lanes > 4) { @@ -5581,8 +5583,8 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, case PIPE_A: return true; case PIPE_B: - if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled && - pipe_config->fdi_lanes > 2) { + if (pipe_config->fdi_lanes > 2 && + pipe_required_fdi_lanes(dev, PIPE_C) > 0) { DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n", pipe_name(pipe), pipe_config->fdi_lanes); return false; @@ -5594,8 +5596,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, pipe_name(pipe), pipe_config->fdi_lanes); return false; } - if (pipe_has_enabled_pch(pipe_B_crtc) && - pipe_B_crtc->config->fdi_lanes > 2) { + if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) { DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n"); return false; } -- 2.0.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks 2015-03-11 16:52 ` [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks ville.syrjala @ 2015-03-12 7:35 ` Ander Conselvan De Oliveira 2015-03-12 9:51 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Ander Conselvan De Oliveira @ 2015-03-12 7:35 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx (for the series) Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> On Wed, 2015-03-11 at 18:52 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Ignore the current state of the pipe and just check crtc_state->enable > and the number of FDI lanes required. This means we don't accidentally > mistake the FDI lanes as being available of one of the pipes just > happens to be disabled at the time of the check. Also we no longer > consider pipe C to require FDI lanes when it's driving the eDP > transcoder. > > We also take the opportunity to make the code a bit nicer looking by > hiding the ugly bits in the new pipe_required_fdi_lanes() function. > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 242a8a7..72e9816 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3152,12 +3152,6 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) > FDI_FE_ERRC_ENABLE); > } > > -static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > -{ > - return crtc->base.state->enable && crtc->active && > - crtc->config->has_pch_encoder; > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -5548,13 +5542,21 @@ bool intel_connector_get_hw_state(struct intel_connector *connector) > return encoder->get_hw_state(encoder, &pipe); > } > > +static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe) > +{ > + struct intel_crtc *crtc = > + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe)); > + > + if (crtc->base.state->enable && > + crtc->config->has_pch_encoder) > + return crtc->config->fdi_lanes; > + > + return 0; > +} > + > static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > struct intel_crtc_state *pipe_config) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *pipe_B_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]); > - > DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n", > pipe_name(pipe), pipe_config->fdi_lanes); > if (pipe_config->fdi_lanes > 4) { > @@ -5581,8 +5583,8 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > case PIPE_A: > return true; > case PIPE_B: > - if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled && > - pipe_config->fdi_lanes > 2) { > + if (pipe_config->fdi_lanes > 2 && > + pipe_required_fdi_lanes(dev, PIPE_C) > 0) { > DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n", > pipe_name(pipe), pipe_config->fdi_lanes); > return false; > @@ -5594,8 +5596,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > pipe_name(pipe), pipe_config->fdi_lanes); > return false; > } > - if (pipe_has_enabled_pch(pipe_B_crtc) && > - pipe_B_crtc->config->fdi_lanes > 2) { > + if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) { > DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n"); > return false; > } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks 2015-03-12 7:35 ` Ander Conselvan De Oliveira @ 2015-03-12 9:51 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2015-03-12 9:51 UTC (permalink / raw) To: Ander Conselvan De Oliveira; +Cc: intel-gfx On Thu, Mar 12, 2015 at 09:35:33AM +0200, Ander Conselvan De Oliveira wrote: > (for the series) > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Both merged to dinq, thanks. -Daniel > > On Wed, 2015-03-11 at 18:52 +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Ignore the current state of the pipe and just check crtc_state->enable > > and the number of FDI lanes required. This means we don't accidentally > > mistake the FDI lanes as being available of one of the pipes just > > happens to be disabled at the time of the check. Also we no longer > > consider pipe C to require FDI lanes when it's driving the eDP > > transcoder. > > > > We also take the opportunity to make the code a bit nicer looking by > > hiding the ugly bits in the new pipe_required_fdi_lanes() function. > > > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++-------------- > > 1 file changed, 15 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 242a8a7..72e9816 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3152,12 +3152,6 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) > > FDI_FE_ERRC_ENABLE); > > } > > > > -static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > > -{ > > - return crtc->base.state->enable && crtc->active && > > - crtc->config->has_pch_encoder; > > -} > > - > > /* The FDI link training functions for ILK/Ibexpeak. */ > > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > > { > > @@ -5548,13 +5542,21 @@ bool intel_connector_get_hw_state(struct intel_connector *connector) > > return encoder->get_hw_state(encoder, &pipe); > > } > > > > +static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe) > > +{ > > + struct intel_crtc *crtc = > > + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe)); > > + > > + if (crtc->base.state->enable && > > + crtc->config->has_pch_encoder) > > + return crtc->config->fdi_lanes; > > + > > + return 0; > > +} > > + > > static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > > struct intel_crtc_state *pipe_config) > > { > > - struct drm_i915_private *dev_priv = dev->dev_private; > > - struct intel_crtc *pipe_B_crtc = > > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]); > > - > > DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n", > > pipe_name(pipe), pipe_config->fdi_lanes); > > if (pipe_config->fdi_lanes > 4) { > > @@ -5581,8 +5583,8 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > > case PIPE_A: > > return true; > > case PIPE_B: > > - if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled && > > - pipe_config->fdi_lanes > 2) { > > + if (pipe_config->fdi_lanes > 2 && > > + pipe_required_fdi_lanes(dev, PIPE_C) > 0) { > > DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n", > > pipe_name(pipe), pipe_config->fdi_lanes); > > return false; > > @@ -5594,8 +5596,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > > pipe_name(pipe), pipe_config->fdi_lanes); > > return false; > > } > > - if (pipe_has_enabled_pch(pipe_B_crtc) && > > - pipe_B_crtc->config->fdi_lanes > 2) { > > + if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) { > > DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n"); > > return false; > > } > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() 2015-03-11 16:52 [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() ville.syrjala 2015-03-11 16:52 ` [PATCH 1/2] drm/i915: Rewrite some some of the FDI lane checks ville.syrjala 2015-03-11 16:52 ` [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks ville.syrjala @ 2015-03-12 7:28 ` Ander Conselvan De Oliveira 2 siblings, 0 replies; 6+ messages in thread From: Ander Conselvan De Oliveira @ 2015-03-12 7:28 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Wed, 2015-03-11 at 18:52 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > So these on top of Ander's latest FDI bifurcation patch [1] should hopefully > result in something sane. Didn't test them though, but assuming Ander > has that test somewhere maybe some of the problematic cases I identified [2] > could be checked with these. I run the tests I wrote here and they all passed. However, the tests don't cover the eDP on pipe C case. Moreover, I don't have an IVB laptop I could use for testing that case. Ander > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061802.html > [2] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061806.html > > Ville Syrjälä (2): > drm/i915: Rewrite some some of the FDI lane checks > drm/i915: Rewrite IVB FDI bifurcation conflict checks > > drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 20 deletions(-) > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-12 9:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-03-11 16:52 [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() ville.syrjala 2015-03-11 16:52 ` [PATCH 1/2] drm/i915: Rewrite some some of the FDI lane checks ville.syrjala 2015-03-11 16:52 ` [PATCH 2/2] drm/i915: Rewrite IVB FDI bifurcation conflict checks ville.syrjala 2015-03-12 7:35 ` Ander Conselvan De Oliveira 2015-03-12 9:51 ` Daniel Vetter 2015-03-12 7:28 ` [PATCH 0/2] drm/i915: Try to sort out ironlake_check_fdi_lanes() Ander Conselvan De Oliveira
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox