* [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C @ 2015-03-09 8:59 Ander Conselvan de Oliveira 2015-03-09 9:24 ` Jani Nikula 2015-03-09 12:17 ` [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C shuang.he 0 siblings, 2 replies; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-09 8:59 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira When enabling pipe C, the check for the number of lanes pipe B uses was ignored in case pipe B wasn't active. This would allow pipe C to be configured while pipe B is in DPMS off state even if it used more than 2 lanes. Making pipe B active again while pipe C was also active would then fail. Tested-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 597c10b..4008bf4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3150,8 +3150,7 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) static bool pipe_has_enabled_pch(struct intel_crtc *crtc) { - return crtc->base.state->enable && crtc->active && - crtc->config->has_pch_encoder; + return crtc->base.state->enable && crtc->config->has_pch_encoder; } static void ivb_modeset_global_resources(struct drm_device *dev) -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C 2015-03-09 8:59 [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C Ander Conselvan de Oliveira @ 2015-03-09 9:24 ` Jani Nikula 2015-03-09 9:33 ` Ander Conselvan De Oliveira 2015-03-09 12:17 ` [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C shuang.he 1 sibling, 1 reply; 27+ messages in thread From: Jani Nikula @ 2015-03-09 9:24 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira On Mon, 09 Mar 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote: > When enabling pipe C, the check for the number of lanes pipe B uses was > ignored in case pipe B wasn't active. This would allow pipe C to be > configured while pipe B is in DPMS off state even if it used more than 2 > lanes. Making pipe B active again while pipe C was also active would > then fail. Seems like a good catch. Broken when, or since forever? Cc: stable? Bugzillas? BR, Jani. > > Tested-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 597c10b..4008bf4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3150,8 +3150,7 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) > > static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > { > - return crtc->base.state->enable && crtc->active && > - crtc->config->has_pch_encoder; > + return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > static void ivb_modeset_global_resources(struct drm_device *dev) > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C 2015-03-09 9:24 ` Jani Nikula @ 2015-03-09 9:33 ` Ander Conselvan De Oliveira 2015-03-09 16:21 ` Daniel Vetter 0 siblings, 1 reply; 27+ messages in thread From: Ander Conselvan De Oliveira @ 2015-03-09 9:33 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Mon, 2015-03-09 at 11:24 +0200, Jani Nikula wrote: > On Mon, 09 Mar 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote: > > When enabling pipe C, the check for the number of lanes pipe B uses was > > ignored in case pipe B wasn't active. This would allow pipe C to be > > configured while pipe B is in DPMS off state even if it used more than 2 > > lanes. Making pipe B active again while pipe C was also active would > > then fail. > > Seems like a good catch. Broken when, or since forever? Cc: stable? > Bugzillas? I had to touch this code in the last patch series I submitted, and I raised a concern that this might do the wrong thing. Daniel suggested a tried the test case I described above, which indeed does fail. I haven't done the actual history digging until a minute ago, which turns out quite interesting. The exact opposite of this patch was done in the following patch: commit 1fbc0d789d12fec313c91912fc11733fdfbab863 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Tue Oct 29 12:04:08 2013 +0100 drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb I'm not sure how much has changed since then, or if the comments on that commit's message are still relevant. Particularly, if the unifying of mode set and dpms on code was ever done, and if it has any effect here. Ander. > > > > Tested-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 597c10b..4008bf4 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3150,8 +3150,7 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc) > > > > static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > > { > > - return crtc->base.state->enable && crtc->active && > > - crtc->config->has_pch_encoder; > > + return crtc->base.state->enable && crtc->config->has_pch_encoder; > > } > > > > static void ivb_modeset_global_resources(struct drm_device *dev) > > -- > > 2.1.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C 2015-03-09 9:33 ` Ander Conselvan De Oliveira @ 2015-03-09 16:21 ` Daniel Vetter 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira 0 siblings, 1 reply; 27+ messages in thread From: Daniel Vetter @ 2015-03-09 16:21 UTC (permalink / raw) To: Ander Conselvan De Oliveira; +Cc: intel-gfx On Mon, Mar 09, 2015 at 11:33:57AM +0200, Ander Conselvan De Oliveira wrote: > On Mon, 2015-03-09 at 11:24 +0200, Jani Nikula wrote: > > On Mon, 09 Mar 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote: > > > When enabling pipe C, the check for the number of lanes pipe B uses was > > > ignored in case pipe B wasn't active. This would allow pipe C to be > > > configured while pipe B is in DPMS off state even if it used more than 2 > > > lanes. Making pipe B active again while pipe C was also active would > > > then fail. > > > > Seems like a good catch. Broken when, or since forever? Cc: stable? > > Bugzillas? > > I had to touch this code in the last patch series I submitted, and I > raised a concern that this might do the wrong thing. Daniel suggested a > tried the test case I described above, which indeed does fail. I haven't > done the actual history digging until a minute ago, which turns out > quite interesting. The exact opposite of this patch was done in the > following patch: > > commit 1fbc0d789d12fec313c91912fc11733fdfbab863 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Tue Oct 29 12:04:08 2013 +0100 > > drm/i915: Fix the PPT fdi lane bifurcate state handling on ivb > > I'm not sure how much has changed since then, or if the comments on that > commit's message are still relevant. Particularly, if the unifying of > mode set and dpms on code was ever done, and if it has any effect here. Indeed your patch would break things again I think for the case. What we essentially want to know is whether we've had to do a modeset change on a given pipe that might require us to update the bifurcate state. We abuse intel->active as a cheap way to tell since for the case we're interested in we have crtc->base.enabled == true and crtc->active == false. That's the case where the pipe will be enabled again, but has been shut down to change the mode. With atomic we need to probably look at crtc_state->mode_changed. As an interim solution we have the same information available in the modeset_pipes bitmask. I think replacing the check for intel_crtc->active with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. The way to reproduce the original bug should be: - Light up pipe B with something requiring more than 2 lanes. - Do an immediate modeset on pipe B to a mode requiring at most 2 lanes. Not that SNA/X always does an interim modeset to everything off, you'd need to code up an igt I think. Or vt-switch between X servers with different modes. Without the intel_crtc->active check we won't set the bifurcate bit. - Try to light up pipe C and go boom on the bifurcate consistency checks. Cheers, Daniel -- 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] 27+ messages in thread
* [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-09 16:21 ` Daniel Vetter @ 2015-03-10 12:32 ` Ander Conselvan de Oliveira 2015-03-10 12:35 ` [PATCH] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-10 12:32 UTC (permalink / raw) To: daniel; +Cc: Ander Conselvan de Oliveira, intel-gfx Remove the global modeset resource function that would disable the bifurcation bit, and instead enable/disable it when enabling or disabling the pch transcoder. The checks before the mode set should ensure that the configuration is valid, so it should be safe to do it so. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote: > With atomic we need to probably look at crtc_state->mode_changed. As an > interim solution we have the same information available in the > modeset_pipes bitmask. I think replacing the check for intel_crtc->active > with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. I looked into that solution, but involves passing modeset_pipes way down into the call stack, so I started looking for a different solution. Do you see any caveat with this approach? Thanks, Ander --- drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++----------------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4008bf4..eca5a41 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc, const struct intel_crtc_state *pipe_config); static void intel_begin_crtc_commit(struct drm_crtc *crtc); static void intel_finish_crtc_commit(struct drm_crtc *crtc); +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, + bool pipe_active); static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe) { @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, enum pipe pipe) { struct drm_device *dev = dev_priv->dev; + struct intel_crtc *crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); uint32_t reg, val; /* FDI relies on the transcoder */ @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE; I915_WRITE(reg, val); } + + if (IS_IVYBRIDGE(dev)) + ivybridge_update_fdi_bc_bifurcation(crtc, false); } static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) return crtc->base.state->enable && crtc->config->has_pch_encoder; } -static void ivb_modeset_global_resources(struct drm_device *dev) -{ - 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]); - struct intel_crtc *pipe_C_crtc = - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); - uint32_t temp; - - /* - * When everything is off disable fdi C so that we could enable fdi B - * with all lanes. Note that we don't care about enabled pipes without - * an enabled pch encoder. - */ - if (!pipe_has_enabled_pch(pipe_B_crtc) && - !pipe_has_enabled_pch(pipe_C_crtc)) { - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); - - temp = I915_READ(SOUTH_CHICKEN1); - temp &= ~FDI_BC_BIFURCATION_SELECT; - DRM_DEBUG_KMS("disabling fdi C rx\n"); - I915_WRITE(SOUTH_CHICKEN1, temp); - } -} - /* The FDI link training functions for ILK/Ibexpeak. */ static void ironlake_fdi_link_train(struct drm_crtc *crtc) { @@ -3834,41 +3815,44 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, I915_READ(VSYNCSHIFT(cpu_transcoder))); } -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool enable) { struct drm_i915_private *dev_priv = dev->dev_private; uint32_t temp; temp = I915_READ(SOUTH_CHICKEN1); - if (temp & FDI_BC_BIFURCATION_SELECT) + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) return; WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); - temp |= FDI_BC_BIFURCATION_SELECT; - DRM_DEBUG_KMS("enabling fdi C rx\n"); + temp &= ~FDI_BC_BIFURCATION_SELECT; + if (enable) + temp |= FDI_BC_BIFURCATION_SELECT; + + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); I915_WRITE(SOUTH_CHICKEN1, temp); POSTING_READ(SOUTH_CHICKEN1); } -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, + bool pipe_active) { struct drm_device *dev = intel_crtc->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; switch (intel_crtc->pipe) { case PIPE_A: break; case PIPE_B: - if (intel_crtc->config->fdi_lanes > 2) - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); + if (intel_crtc->config->fdi_lanes > 2 && pipe_active) + cpt_enable_fdi_bc_bifurcation(dev, false); else - cpt_enable_fdi_bc_bifurcation(dev); + cpt_enable_fdi_bc_bifurcation(dev, true); break; case PIPE_C: - cpt_enable_fdi_bc_bifurcation(dev); + cpt_enable_fdi_bc_bifurcation(dev, pipe_active); break; default: @@ -3895,7 +3879,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc) assert_pch_transcoder_disabled(dev_priv, pipe); if (IS_IVYBRIDGE(dev)) - ivybridge_update_fdi_bc_bifurcation(intel_crtc); + ivybridge_update_fdi_bc_bifurcation(intel_crtc, true); /* Write the TU size bits before fdi link training, so that error * detection works. */ @@ -13056,8 +13040,6 @@ static void intel_init_display(struct drm_device *dev) } else if (IS_IVYBRIDGE(dev)) { /* FIXME: detect B0+ stepping and use auto training */ dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; - dev_priv->display.modeset_global_resources = - ivb_modeset_global_resources; } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { dev_priv->display.fdi_link_train = hsw_fdi_link_train; } else if (IS_VALLEYVIEW(dev)) { -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH] tests: Add test for pipe B and C interactions in IVB 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira @ 2015-03-10 12:35 ` Ander Conselvan de Oliveira 2015-03-10 19:05 ` Daniel Vetter 2015-03-10 13:03 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ville Syrjälä ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-10 12:35 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira This actually only works if the machine is setup properly. It requires that: - the hardware to have at least two connected outpus; - the defatult mode on those outputs needs to be big enough; - the attached monitors need to support 10 bpc. This might generate spurius results if the connected output doesn't support 10bpc or a large enough mode. --- I used this to test the changes that affect pipe B and C interactions on IVB. I'm not really sure how to turn into a proper igt test though, because of the requirements on the outputs in use. --- tests/Makefile.sources | 1 + tests/kms_pipe_b_c_dpms.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/kms_pipe_b_c_dpms.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 9ab0ff4..9e43a64 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -73,6 +73,7 @@ TESTS_progs_M = \ kms_nuclear \ kms_pipe_crc_basic \ kms_plane \ + kms_pipe_b_c_dpms \ kms_psr_sink_crc \ kms_render \ kms_rotation_crc \ diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c new file mode 100644 index 0000000..a5ed761 --- /dev/null +++ b/tests/kms_pipe_b_c_dpms.c @@ -0,0 +1,173 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> + */ + +#include "drmtest.h" +#include "igt_kms.h" +#include "intel_chipset.h" + +typedef struct { + int drm_fd; + igt_display_t display; +} data_t; + +static int +set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output, int bpc) +{ + igt_plane_t *primary; + drmModeModeInfo *mode; + struct igt_fb fb; + int fb_id; + uint32_t format; + + if (bpc == 8) + format = DRM_FORMAT_XRGB8888; + else if (bpc == 10) + format = DRM_FORMAT_XRGB2101010; + else + igt_fail(1); + + igt_output_set_pipe(output, pipe); + + /* FIXME: we need a big mode */ + mode = igt_output_get_mode(output); + primary = igt_output_get_plane(output, 0); + + fb_id = igt_create_color_fb(data->drm_fd, + mode->hdisplay, mode->vdisplay, + format, I915_TILING_NONE, + 1.0, 1.0, 1.0, &fb); + igt_assert(fb_id >= 0); + + igt_plane_set_fb(primary, &fb); + return igt_display_try_commit2(&data->display, COMMIT_LEGACY); +} + +static int +set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + return set_mode_on_pipe(data, pipe, output, 10); +} + +static int +set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + return set_mode_on_pipe(data, pipe, output, 8); +} + +static void +find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2) +{ + int count = 0; + igt_output_t *output; + + *output1 = NULL; + *output2 = NULL; + + for_each_connected_output(&data->display, output) { + if (!(*output1)) + *output1 = output; + else if (!(*output2)) + *output2 = output; + + igt_output_set_pipe(output, PIPE_ANY); + count++; + } + + igt_skip_on_f(count < 2, "Not enough connected outputs\n"); +} + +static void +test_dpms(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF); + + ret = set_big_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret != 0); +} + +static void +test_lane_reduction(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret == 0); +} + +static data_t data; +igt_main +{ + int devid; + + igt_skip_on_simulation(); + + igt_fixture { + data.drm_fd = drm_open_any_master(); + devid = intel_get_drm_devid(data.drm_fd); + + kmstest_set_vt_graphics_mode(); + igt_display_init(&data.display, data.drm_fd); + } + + igt_skip_on(!IS_IVYBRIDGE(devid)); + + igt_subtest("pipe-B-dpms-off-modeset-pipe-C") + test_dpms(&data); + + igt_subtest("pipe-B-double-modeset-then-modeset-pipe-C") + test_lane_reduction(&data); + + igt_fixture { + igt_display_fini(&data.display); + } +} -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] tests: Add test for pipe B and C interactions in IVB 2015-03-10 12:35 ` [PATCH] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira @ 2015-03-10 19:05 ` Daniel Vetter 2015-03-11 11:33 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Ander Conselvan de Oliveira 0 siblings, 1 reply; 27+ messages in thread From: Daniel Vetter @ 2015-03-10 19:05 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Tue, Mar 10, 2015 at 02:35:56PM +0200, Ander Conselvan de Oliveira wrote: > This actually only works if the machine is setup properly. It requires > that: > > - the hardware to have at least two connected outpus; > - the defatult mode on those outputs needs to be big enough; > - the attached monitors need to support 10 bpc. > > This might generate spurius results if the connected output doesn't support > 10bpc or a large enough mode. > > --- > I used this to test the changes that affect pipe B and C interactions on > IVB. I'm not really sure how to turn into a proper igt test though, because > of the requirements on the outputs in use. Sprinkle lots of igt_require(condition) over the testcase. In this case a bit tricky since we'd need to look at the edid to check that there's a 10bpc screeen. Maybe use as an alternative plan just a higher resolution and only check for the fdi per-lane dotclock limit? That should be more reliable since for external outputs we start out with at least 8bpc everywhere before deciding to dither down. -Daniel > > > --- > tests/Makefile.sources | 1 + > tests/kms_pipe_b_c_dpms.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 174 insertions(+) > create mode 100644 tests/kms_pipe_b_c_dpms.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 9ab0ff4..9e43a64 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -73,6 +73,7 @@ TESTS_progs_M = \ > kms_nuclear \ > kms_pipe_crc_basic \ > kms_plane \ > + kms_pipe_b_c_dpms \ > kms_psr_sink_crc \ > kms_render \ > kms_rotation_crc \ > diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c > new file mode 100644 > index 0000000..a5ed761 > --- /dev/null > +++ b/tests/kms_pipe_b_c_dpms.c > @@ -0,0 +1,173 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > + */ > + > +#include "drmtest.h" > +#include "igt_kms.h" > +#include "intel_chipset.h" > + > +typedef struct { > + int drm_fd; > + igt_display_t display; > +} data_t; > + > +static int > +set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output, int bpc) > +{ > + igt_plane_t *primary; > + drmModeModeInfo *mode; > + struct igt_fb fb; > + int fb_id; > + uint32_t format; > + > + if (bpc == 8) > + format = DRM_FORMAT_XRGB8888; > + else if (bpc == 10) > + format = DRM_FORMAT_XRGB2101010; > + else > + igt_fail(1); > + > + igt_output_set_pipe(output, pipe); > + > + /* FIXME: we need a big mode */ > + mode = igt_output_get_mode(output); > + primary = igt_output_get_plane(output, 0); > + > + fb_id = igt_create_color_fb(data->drm_fd, > + mode->hdisplay, mode->vdisplay, > + format, I915_TILING_NONE, > + 1.0, 1.0, 1.0, &fb); > + igt_assert(fb_id >= 0); > + > + igt_plane_set_fb(primary, &fb); > + return igt_display_try_commit2(&data->display, COMMIT_LEGACY); > +} > + > +static int > +set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + return set_mode_on_pipe(data, pipe, output, 10); > +} > + > +static int > +set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + return set_mode_on_pipe(data, pipe, output, 8); > +} > + > +static void > +find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2) > +{ > + int count = 0; > + igt_output_t *output; > + > + *output1 = NULL; > + *output2 = NULL; > + > + for_each_connected_output(&data->display, output) { > + if (!(*output1)) > + *output1 = output; > + else if (!(*output2)) > + *output2 = output; > + > + igt_output_set_pipe(output, PIPE_ANY); > + count++; > + } > + > + igt_skip_on_f(count < 2, "Not enough connected outputs\n"); > +} > + > +static void > +test_dpms(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF); > + > + ret = set_big_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret != 0); > +} > + > +static void > +test_lane_reduction(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret == 0); > +} > + > +static data_t data; > +igt_main > +{ > + int devid; > + > + igt_skip_on_simulation(); > + > + igt_fixture { > + data.drm_fd = drm_open_any_master(); > + devid = intel_get_drm_devid(data.drm_fd); > + > + kmstest_set_vt_graphics_mode(); > + igt_display_init(&data.display, data.drm_fd); > + } > + > + igt_skip_on(!IS_IVYBRIDGE(devid)); > + > + igt_subtest("pipe-B-dpms-off-modeset-pipe-C") > + test_dpms(&data); > + > + igt_subtest("pipe-B-double-modeset-then-modeset-pipe-C") > + test_lane_reduction(&data); > + > + igt_fixture { > + igt_display_fini(&data.display); > + } > +} > -- > 2.1.0 > > _______________________________________________ > 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] 27+ messages in thread
* [PATCH igt 1/2] lib/kms: Add a way to override an output's mode 2015-03-10 19:05 ` Daniel Vetter @ 2015-03-11 11:33 ` Ander Conselvan de Oliveira 2015-03-11 11:33 ` [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-11 11:33 UTC (permalink / raw) To: daniel; +Cc: Ander Conselvan de Oliveira, intel-gfx So that it is possible to use a custom mode with the simplified mode set API. --- lib/igt_kms.c | 9 +++++++++ lib/igt_kms.h | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 26e4913..0dccd2d 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -895,6 +895,9 @@ static void igt_output_refresh(igt_output_t *output) if (!output->valid) return; + if (output->use_override_mode) + output->config.default_mode = output->override_mode; + if (!output->name) { drmModeConnector *c = output->config.connector; @@ -1797,6 +1800,12 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output) return &output->config.default_mode; } +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode) +{ + output->override_mode = *mode; + output->use_override_mode = true; +} + void igt_output_set_pipe(igt_output_t *output, enum pipe pipe) { igt_display_t *display = output->display; diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 2fab30e..ddf4432 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -228,6 +228,9 @@ typedef struct { bool valid; unsigned long pending_crtc_idx_mask; + bool use_override_mode; + drmModeModeInfo override_mode; + #ifdef HAVE_ATOMIC /* Property set for nuclear pageflip */ drmModePropertySetPtr set; @@ -255,6 +258,7 @@ int igt_display_get_n_pipes(igt_display_t *display); const char *igt_output_name(igt_output_t *output); drmModeModeInfo *igt_output_get_mode(igt_output_t *output); +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode); void igt_output_set_pipe(igt_output_t *output, enum pipe pipe); igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane); -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB 2015-03-11 11:33 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Ander Conselvan de Oliveira @ 2015-03-11 11:33 ` Ander Conselvan de Oliveira 2015-03-27 13:35 ` Thomas Wood 2015-03-11 13:26 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Damien Lespiau 2015-03-27 13:30 ` Thomas Wood 2 siblings, 1 reply; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-11 11:33 UTC (permalink / raw) To: daniel; +Cc: Ander Conselvan de Oliveira, intel-gfx The tests exercise different combinations of enabling pipe B with modes that require more than 2 lanes and then enabling pipe C. v2: Added a couple more tests for different pipe transitions. (Ander) Use custom modes to make the test reliable. (Daniel) --- tests/Makefile.sources | 1 + tests/kms_pipe_b_c_dpms.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+) create mode 100644 tests/kms_pipe_b_c_dpms.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 9ab0ff4..9e43a64 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -73,6 +73,7 @@ TESTS_progs_M = \ kms_nuclear \ kms_pipe_crc_basic \ kms_plane \ + kms_pipe_b_c_dpms \ kms_psr_sink_crc \ kms_render \ kms_rotation_crc \ diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c new file mode 100644 index 0000000..69394f4 --- /dev/null +++ b/tests/kms_pipe_b_c_dpms.c @@ -0,0 +1,286 @@ +/* + * Copyright © 2015 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> + */ + +#include "drmtest.h" +#include "igt_kms.h" +#include "intel_chipset.h" + +typedef struct { + int drm_fd; + igt_display_t display; +} data_t; + +drmModeModeInfo mode_3_lanes = { + .clock = 173000, + .hdisplay = 1920, + .hsync_start = 2048, + .hsync_end = 2248, + .htotal = 2576, + .vdisplay = 1080, + .vsync_start = 1083, + .vsync_end = 1088, + .vtotal = 1120, + .vrefresh = 60, + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, + .name = "3_lanes", +}; + +drmModeModeInfo mode_2_lanes = { + .clock = 138500, + .hdisplay = 1920, + .hsync_start = 1968, + .hsync_end = 2000, + .htotal = 2080, + .vdisplay = 1080, + .vsync_start = 1083, + .vsync_end = 1088, + .vtotal = 1111, + .vrefresh = 60, + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC, + .name = "2_lanes", +}; + +static int +disable_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + igt_plane_t *primary; + + igt_output_set_pipe(output, pipe); + primary = igt_output_get_plane(output, 0); + igt_plane_set_fb(primary, NULL); + return igt_display_commit(&data->display); +} + +static int +set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + igt_plane_t *primary; + drmModeModeInfo *mode; + struct igt_fb fb; + int fb_id; + + igt_output_set_pipe(output, pipe); + + mode = igt_output_get_mode(output); + + primary = igt_output_get_plane(output, 0); + + fb_id = igt_create_color_fb(data->drm_fd, + mode->hdisplay, mode->vdisplay, + DRM_FORMAT_XRGB8888, I915_TILING_NONE, + 1.0, 1.0, 1.0, &fb); + igt_assert(fb_id >= 0); + + igt_plane_set_fb(primary, &fb); + return igt_display_try_commit2(&data->display, COMMIT_LEGACY); +} + +static int +set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + igt_output_override_mode(output, &mode_3_lanes); + return set_mode_on_pipe(data, pipe, output); +} + +static int +set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) +{ + igt_output_override_mode(output, &mode_2_lanes); + return set_mode_on_pipe(data, pipe, output); +} + +static void +find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2) +{ + int count = 0; + igt_output_t *output; + + *output1 = NULL; + *output2 = NULL; + + for_each_connected_output(&data->display, output) { + if (!(*output1)) + *output1 = output; + else if (!(*output2)) + *output2 = output; + + igt_output_set_pipe(output, PIPE_ANY); + count++; + } + + igt_skip_on_f(count < 2, "Not enough connected outputs\n"); +} + +static void +test_dpms(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF); + + ret = set_big_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret != 0); +} + +static void +test_lane_reduction(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret == 0); +} + +static void +test_disable_pipe_B(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = disable_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); +} + +static void +test_from_C_to_B_with_3_lanes(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret == 0); + + ret = disable_pipe(data, PIPE_C, output2); + igt_assert(ret == 0); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); +} + +static void +test_fail_enable_pipe_C_while_B_has_3_lanes(data_t *data) +{ + igt_output_t *output1, *output2; + int ret; + + find_outputs(data, &output1, &output2); + + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); + igt_info("Pipe %s will use connector %s\n", + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); + + ret = set_big_mode_on_pipe(data, PIPE_B, output1); + igt_assert(ret == 0); + + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); + igt_assert(ret != 0); +} + +static data_t data; +igt_main +{ + int devid; + + igt_skip_on_simulation(); + + igt_fixture { + data.drm_fd = drm_open_any_master(); + devid = intel_get_drm_devid(data.drm_fd); + + kmstest_set_vt_graphics_mode(); + igt_display_init(&data.display, data.drm_fd); + } + + igt_skip_on(!IS_IVYBRIDGE(devid)); + + igt_subtest("pipe-B-dpms-off-modeset-pipe-C") + test_dpms(&data); + + igt_subtest("pipe-B-double-modeset-then-modeset-pipe-C") + test_lane_reduction(&data); + + igt_subtest("disable-pipe-B-enable-pipe-C") + test_disable_pipe_B(&data); + + igt_subtest("from-pipe-C-to-B-with-3-lanes") + test_from_C_to_B_with_3_lanes(&data); + + igt_subtest("enable-pipe-C-while-B-has-3-lanes") + test_fail_enable_pipe_C_while_B_has_3_lanes(&data); + + igt_fixture { + igt_display_fini(&data.display); + } +} -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB 2015-03-11 11:33 ` [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira @ 2015-03-27 13:35 ` Thomas Wood 0 siblings, 0 replies; 27+ messages in thread From: Thomas Wood @ 2015-03-27 13:35 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: Intel Graphics Development On 11 March 2015 at 11:33, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote: > The tests exercise different combinations of enabling pipe B with modes > that require more than 2 lanes and then enabling pipe C. > > v2: Added a couple more tests for different pipe transitions. (Ander) > Use custom modes to make the test reliable. (Daniel) > --- > tests/Makefile.sources | 1 + > tests/kms_pipe_b_c_dpms.c | 286 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 287 insertions(+) > create mode 100644 tests/kms_pipe_b_c_dpms.c Please add the test binary to .gitignore and add a short description of the test using the IGT_TEST_DESCRIPTION macro. > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 9ab0ff4..9e43a64 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -73,6 +73,7 @@ TESTS_progs_M = \ > kms_nuclear \ > kms_pipe_crc_basic \ > kms_plane \ > + kms_pipe_b_c_dpms \ > kms_psr_sink_crc \ > kms_render \ > kms_rotation_crc \ > diff --git a/tests/kms_pipe_b_c_dpms.c b/tests/kms_pipe_b_c_dpms.c > new file mode 100644 > index 0000000..69394f4 > --- /dev/null > +++ b/tests/kms_pipe_b_c_dpms.c > @@ -0,0 +1,286 @@ > +/* > + * Copyright © 2015 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > + */ > + > +#include "drmtest.h" > +#include "igt_kms.h" > +#include "intel_chipset.h" > + > +typedef struct { > + int drm_fd; > + igt_display_t display; > +} data_t; > + > +drmModeModeInfo mode_3_lanes = { > + .clock = 173000, > + .hdisplay = 1920, > + .hsync_start = 2048, > + .hsync_end = 2248, > + .htotal = 2576, > + .vdisplay = 1080, > + .vsync_start = 1083, > + .vsync_end = 1088, > + .vtotal = 1120, > + .vrefresh = 60, > + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC, > + .name = "3_lanes", > +}; > + > +drmModeModeInfo mode_2_lanes = { > + .clock = 138500, > + .hdisplay = 1920, > + .hsync_start = 1968, > + .hsync_end = 2000, > + .htotal = 2080, > + .vdisplay = 1080, > + .vsync_start = 1083, > + .vsync_end = 1088, > + .vtotal = 1111, > + .vrefresh = 60, > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC, > + .name = "2_lanes", > +}; > + > +static int > +disable_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + igt_plane_t *primary; > + > + igt_output_set_pipe(output, pipe); > + primary = igt_output_get_plane(output, 0); > + igt_plane_set_fb(primary, NULL); > + return igt_display_commit(&data->display); > +} > + > +static int > +set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + igt_plane_t *primary; > + drmModeModeInfo *mode; > + struct igt_fb fb; > + int fb_id; > + > + igt_output_set_pipe(output, pipe); > + > + mode = igt_output_get_mode(output); > + > + primary = igt_output_get_plane(output, 0); > + > + fb_id = igt_create_color_fb(data->drm_fd, > + mode->hdisplay, mode->vdisplay, > + DRM_FORMAT_XRGB8888, I915_TILING_NONE, > + 1.0, 1.0, 1.0, &fb); > + igt_assert(fb_id >= 0); > + > + igt_plane_set_fb(primary, &fb); > + return igt_display_try_commit2(&data->display, COMMIT_LEGACY); > +} > + > +static int > +set_big_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + igt_output_override_mode(output, &mode_3_lanes); > + return set_mode_on_pipe(data, pipe, output); > +} > + > +static int > +set_normal_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output) > +{ > + igt_output_override_mode(output, &mode_2_lanes); > + return set_mode_on_pipe(data, pipe, output); > +} > + > +static void > +find_outputs(data_t *data, igt_output_t **output1, igt_output_t **output2) > +{ > + int count = 0; > + igt_output_t *output; > + > + *output1 = NULL; > + *output2 = NULL; > + > + for_each_connected_output(&data->display, output) { > + if (!(*output1)) > + *output1 = output; > + else if (!(*output2)) > + *output2 = output; > + > + igt_output_set_pipe(output, PIPE_ANY); > + count++; > + } > + > + igt_skip_on_f(count < 2, "Not enough connected outputs\n"); > +} > + > +static void > +test_dpms(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF); > + > + ret = set_big_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret != 0); > +} > + > +static void > +test_lane_reduction(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret == 0); > +} > + > +static void > +test_disable_pipe_B(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = disable_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > +} > + > +static void > +test_from_C_to_B_with_3_lanes(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret == 0); > + > + ret = disable_pipe(data, PIPE_C, output2); > + igt_assert(ret == 0); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > +} > + > +static void > +test_fail_enable_pipe_C_while_B_has_3_lanes(data_t *data) > +{ > + igt_output_t *output1, *output2; > + int ret; > + > + find_outputs(data, &output1, &output2); > + > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_B), igt_output_name(output1)); > + igt_info("Pipe %s will use connector %s\n", > + kmstest_pipe_name(PIPE_C), igt_output_name(output2)); > + > + ret = set_big_mode_on_pipe(data, PIPE_B, output1); > + igt_assert(ret == 0); > + > + ret = set_normal_mode_on_pipe(data, PIPE_C, output2); > + igt_assert(ret != 0); > +} > + > +static data_t data; > +igt_main > +{ > + int devid; > + > + igt_skip_on_simulation(); > + > + igt_fixture { > + data.drm_fd = drm_open_any_master(); > + devid = intel_get_drm_devid(data.drm_fd); > + > + kmstest_set_vt_graphics_mode(); > + igt_display_init(&data.display, data.drm_fd); > + } > + > + igt_skip_on(!IS_IVYBRIDGE(devid)); > + > + igt_subtest("pipe-B-dpms-off-modeset-pipe-C") > + test_dpms(&data); > + > + igt_subtest("pipe-B-double-modeset-then-modeset-pipe-C") > + test_lane_reduction(&data); > + > + igt_subtest("disable-pipe-B-enable-pipe-C") > + test_disable_pipe_B(&data); > + > + igt_subtest("from-pipe-C-to-B-with-3-lanes") > + test_from_C_to_B_with_3_lanes(&data); > + > + igt_subtest("enable-pipe-C-while-B-has-3-lanes") > + test_fail_enable_pipe_C_while_B_has_3_lanes(&data); > + > + igt_fixture { > + igt_display_fini(&data.display); > + } > +} > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH igt 1/2] lib/kms: Add a way to override an output's mode 2015-03-11 11:33 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Ander Conselvan de Oliveira 2015-03-11 11:33 ` [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira @ 2015-03-11 13:26 ` Damien Lespiau 2015-03-11 13:48 ` Ander Conselvan De Oliveira 2015-03-27 13:30 ` Thomas Wood 2 siblings, 1 reply; 27+ messages in thread From: Damien Lespiau @ 2015-03-11 13:26 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Wed, Mar 11, 2015 at 01:33:00PM +0200, Ander Conselvan de Oliveira wrote: > So that it is possible to use a custom mode with the simplified mode set API. Maybe just igt_output_set_mode()? -- Damien > --- > lib/igt_kms.c | 9 +++++++++ > lib/igt_kms.h | 4 ++++ > 2 files changed, 13 insertions(+) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 26e4913..0dccd2d 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -895,6 +895,9 @@ static void igt_output_refresh(igt_output_t *output) > if (!output->valid) > return; > > + if (output->use_override_mode) > + output->config.default_mode = output->override_mode; > + > if (!output->name) { > drmModeConnector *c = output->config.connector; > > @@ -1797,6 +1800,12 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output) > return &output->config.default_mode; > } > > +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode) > +{ > + output->override_mode = *mode; > + output->use_override_mode = true; > +} > + > void igt_output_set_pipe(igt_output_t *output, enum pipe pipe) > { > igt_display_t *display = output->display; > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > index 2fab30e..ddf4432 100644 > --- a/lib/igt_kms.h > +++ b/lib/igt_kms.h > @@ -228,6 +228,9 @@ typedef struct { > bool valid; > unsigned long pending_crtc_idx_mask; > > + bool use_override_mode; > + drmModeModeInfo override_mode; > + > #ifdef HAVE_ATOMIC > /* Property set for nuclear pageflip */ > drmModePropertySetPtr set; > @@ -255,6 +258,7 @@ int igt_display_get_n_pipes(igt_display_t *display); > > const char *igt_output_name(igt_output_t *output); > drmModeModeInfo *igt_output_get_mode(igt_output_t *output); > +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode); > void igt_output_set_pipe(igt_output_t *output, enum pipe pipe); > igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane); > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH igt 1/2] lib/kms: Add a way to override an output's mode 2015-03-11 13:26 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Damien Lespiau @ 2015-03-11 13:48 ` Ander Conselvan De Oliveira 2015-03-11 14:26 ` Damien Lespiau 0 siblings, 1 reply; 27+ messages in thread From: Ander Conselvan De Oliveira @ 2015-03-11 13:48 UTC (permalink / raw) To: Damien Lespiau; +Cc: intel-gfx On Wed, 2015-03-11 at 13:26 +0000, Damien Lespiau wrote: > On Wed, Mar 11, 2015 at 01:33:00PM +0200, Ander Conselvan de Oliveira wrote: > > So that it is possible to use a custom mode with the simplified mode set API. > > Maybe just igt_output_set_mode()? That works too. I used override since there's a chance the desired mode won't produce the expected results. But now that I think about it "force mode" would sound more like that. In any case, I don't mind either way. Ander _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH igt 1/2] lib/kms: Add a way to override an output's mode 2015-03-11 13:48 ` Ander Conselvan De Oliveira @ 2015-03-11 14:26 ` Damien Lespiau 0 siblings, 0 replies; 27+ messages in thread From: Damien Lespiau @ 2015-03-11 14:26 UTC (permalink / raw) To: Ander Conselvan De Oliveira; +Cc: intel-gfx On Wed, Mar 11, 2015 at 03:48:00PM +0200, Ander Conselvan De Oliveira wrote: > On Wed, 2015-03-11 at 13:26 +0000, Damien Lespiau wrote: > > On Wed, Mar 11, 2015 at 01:33:00PM +0200, Ander Conselvan de Oliveira wrote: > > > So that it is possible to use a custom mode with the simplified mode set API. > > > > Maybe just igt_output_set_mode()? > > That works too. I used override since there's a chance the desired mode > won't produce the expected results. But now that I think about it "force > mode" would sound more like that. In any case, I don't mind either way. No, me neither, can go as is anyway. -- Damien _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH igt 1/2] lib/kms: Add a way to override an output's mode 2015-03-11 11:33 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Ander Conselvan de Oliveira 2015-03-11 11:33 ` [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira 2015-03-11 13:26 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Damien Lespiau @ 2015-03-27 13:30 ` Thomas Wood 2 siblings, 0 replies; 27+ messages in thread From: Thomas Wood @ 2015-03-27 13:30 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: Intel Graphics Development On 11 March 2015 at 11:33, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> wrote: > So that it is possible to use a custom mode with the simplified mode set API. > --- > lib/igt_kms.c | 9 +++++++++ > lib/igt_kms.h | 4 ++++ > 2 files changed, 13 insertions(+) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 26e4913..0dccd2d 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -895,6 +895,9 @@ static void igt_output_refresh(igt_output_t *output) > if (!output->valid) > return; > > + if (output->use_override_mode) > + output->config.default_mode = output->override_mode; > + > if (!output->name) { > drmModeConnector *c = output->config.connector; > > @@ -1797,6 +1800,12 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output) > return &output->config.default_mode; > } > Please add some API documentation for the new function here. > +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode) > +{ > + output->override_mode = *mode; > + output->use_override_mode = true; > +} > + > void igt_output_set_pipe(igt_output_t *output, enum pipe pipe) > { > igt_display_t *display = output->display; > diff --git a/lib/igt_kms.h b/lib/igt_kms.h > index 2fab30e..ddf4432 100644 > --- a/lib/igt_kms.h > +++ b/lib/igt_kms.h > @@ -228,6 +228,9 @@ typedef struct { > bool valid; > unsigned long pending_crtc_idx_mask; > > + bool use_override_mode; > + drmModeModeInfo override_mode; > + > #ifdef HAVE_ATOMIC > /* Property set for nuclear pageflip */ > drmModePropertySetPtr set; > @@ -255,6 +258,7 @@ int igt_display_get_n_pipes(igt_display_t *display); > > const char *igt_output_name(igt_output_t *output); > drmModeModeInfo *igt_output_get_mode(igt_output_t *output); > +void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode); > void igt_output_set_pipe(igt_output_t *output, enum pipe pipe); > igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane); > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira 2015-03-10 12:35 ` [PATCH] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira @ 2015-03-10 13:03 ` Ville Syrjälä 2015-03-10 19:14 ` Daniel Vetter 2015-03-10 19:10 ` Daniel Vetter 2015-03-10 20:40 ` shuang.he 3 siblings, 1 reply; 27+ messages in thread From: Ville Syrjälä @ 2015-03-10 13:03 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote: > Remove the global modeset resource function that would disable the > bifurcation bit, and instead enable/disable it when enabling or > disabling the pch transcoder. The checks before the mode set should > ensure that the configuration is valid, so it should be safe to do it > so. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > > On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote: > > With atomic we need to probably look at crtc_state->mode_changed. As an > > interim solution we have the same information available in the > > modeset_pipes bitmask. I think replacing the check for intel_crtc->active > > with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. > > I looked into that solution, but involves passing modeset_pipes way down > into the call stack, so I started looking for a different solution. Do you > see any caveat with this approach? > > Thanks, > Ander > > --- > drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++----------------------- > 1 file changed, 21 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4008bf4..eca5a41 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc, > const struct intel_crtc_state *pipe_config); > static void intel_begin_crtc_commit(struct drm_crtc *crtc); > static void intel_finish_crtc_commit(struct drm_crtc *crtc); > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > + bool pipe_active); > > static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe) > { > @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > enum pipe pipe) > { > struct drm_device *dev = dev_priv->dev; > + struct intel_crtc *crtc = > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > uint32_t reg, val; > > /* FDI relies on the transcoder */ > @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE; > I915_WRITE(reg, val); > } > + > + if (IS_IVYBRIDGE(dev)) > + ivybridge_update_fdi_bc_bifurcation(crtc, false); > } > > static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) > @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > -static void ivb_modeset_global_resources(struct drm_device *dev) > -{ > - 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]); > - struct intel_crtc *pipe_C_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > - uint32_t temp; > - > - /* > - * When everything is off disable fdi C so that we could enable fdi B > - * with all lanes. Note that we don't care about enabled pipes without > - * an enabled pch encoder. > - */ > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > - !pipe_has_enabled_pch(pipe_C_crtc)) { > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > - > - temp = I915_READ(SOUTH_CHICKEN1); > - temp &= ~FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > - I915_WRITE(SOUTH_CHICKEN1, temp); > - } > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -3834,41 +3815,44 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > I915_READ(VSYNCSHIFT(cpu_transcoder))); > } > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t temp; > > temp = I915_READ(SOUTH_CHICKEN1); > - if (temp & FDI_BC_BIFURCATION_SELECT) > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > return; > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - temp |= FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > + temp &= ~FDI_BC_BIFURCATION_SELECT; > + if (enable) > + temp |= FDI_BC_BIFURCATION_SELECT; > + > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > I915_WRITE(SOUTH_CHICKEN1, temp); > POSTING_READ(SOUTH_CHICKEN1); > } > > -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > + bool pipe_active) > { > struct drm_device *dev = intel_crtc->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > > switch (intel_crtc->pipe) { > case PIPE_A: > break; > case PIPE_B: > - if (intel_crtc->config->fdi_lanes > 2) > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > + if (intel_crtc->config->fdi_lanes > 2 && pipe_active) > + cpt_enable_fdi_bc_bifurcation(dev, false); > else > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_enable_fdi_bc_bifurcation(dev, true); > > break; > case PIPE_C: > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_enable_fdi_bc_bifurcation(dev, pipe_active); > It sees could now change the bifurcation while pipe B is active (but only using two lanes). Not sure if the old code did that too, or if it might cause problems. Also depending on the order you disable the pipes you might end up with bifurcation enabled or disabled. It seems to me that the simplest solution should be to have bifurcation enabled at all times, except when pipe B really needs the four lanes. Then the hardware state would only need to be changed when enabling/disabling pipe B with four lanes. Rest of the time crtc->enabled and fdi_lanes should be able to tell us which configuration is possible and which not. Or am I missing something? Well, eventually we want to tie this into the atomic state so that we can actaully have pipe B drop into two lane mode if pipe C needs the lanes (and pipe B can still operate with two lanes). But I guess that's still not achievable with the current code. > break; > default: > @@ -3895,7 +3879,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc) > assert_pch_transcoder_disabled(dev_priv, pipe); > > if (IS_IVYBRIDGE(dev)) > - ivybridge_update_fdi_bc_bifurcation(intel_crtc); > + ivybridge_update_fdi_bc_bifurcation(intel_crtc, true); > > /* Write the TU size bits before fdi link training, so that error > * detection works. */ > @@ -13056,8 +13040,6 @@ static void intel_init_display(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > - dev_priv->display.modeset_global_resources = > - ivb_modeset_global_resources; > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > } else if (IS_VALLEYVIEW(dev)) { > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-10 13:03 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ville Syrjälä @ 2015-03-10 19:14 ` Daniel Vetter 2015-03-11 11:35 ` Ander Conselvan de Oliveira 0 siblings, 1 reply; 27+ messages in thread From: Daniel Vetter @ 2015-03-10 19:14 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Ander Conselvan de Oliveira, intel-gfx On Tue, Mar 10, 2015 at 03:03:59PM +0200, Ville Syrjälä wrote: > On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote: > > Remove the global modeset resource function that would disable the > > bifurcation bit, and instead enable/disable it when enabling or > > disabling the pch transcoder. The checks before the mode set should > > ensure that the configuration is valid, so it should be safe to do it > > so. > > > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > --- > > > > On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote: > > > With atomic we need to probably look at crtc_state->mode_changed. As an > > > interim solution we have the same information available in the > > > modeset_pipes bitmask. I think replacing the check for intel_crtc->active > > > with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. > > > > I looked into that solution, but involves passing modeset_pipes way down > > into the call stack, so I started looking for a different solution. Do you > > see any caveat with this approach? > > > > Thanks, > > Ander > > > > --- > > drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++----------------------- > > 1 file changed, 21 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 4008bf4..eca5a41 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc, > > const struct intel_crtc_state *pipe_config); > > static void intel_begin_crtc_commit(struct drm_crtc *crtc); > > static void intel_finish_crtc_commit(struct drm_crtc *crtc); > > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > > + bool pipe_active); > > > > static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe) > > { > > @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > > enum pipe pipe) > > { > > struct drm_device *dev = dev_priv->dev; > > + struct intel_crtc *crtc = > > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > uint32_t reg, val; > > > > /* FDI relies on the transcoder */ > > @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > > val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE; > > I915_WRITE(reg, val); > > } > > + > > + if (IS_IVYBRIDGE(dev)) > > + ivybridge_update_fdi_bc_bifurcation(crtc, false); > > } > > > > static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) > > @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > > return crtc->base.state->enable && crtc->config->has_pch_encoder; > > } > > > > -static void ivb_modeset_global_resources(struct drm_device *dev) > > -{ > > - 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]); > > - struct intel_crtc *pipe_C_crtc = > > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > > - uint32_t temp; > > - > > - /* > > - * When everything is off disable fdi C so that we could enable fdi B > > - * with all lanes. Note that we don't care about enabled pipes without > > - * an enabled pch encoder. > > - */ > > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > > - !pipe_has_enabled_pch(pipe_C_crtc)) { > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - > > - temp = I915_READ(SOUTH_CHICKEN1); > > - temp &= ~FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > > - I915_WRITE(SOUTH_CHICKEN1, temp); > > - } > > -} > > - > > /* The FDI link training functions for ILK/Ibexpeak. */ > > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > > { > > @@ -3834,41 +3815,44 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > > I915_READ(VSYNCSHIFT(cpu_transcoder))); > > } > > > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > > +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > uint32_t temp; > > > > temp = I915_READ(SOUTH_CHICKEN1); > > - if (temp & FDI_BC_BIFURCATION_SELECT) > > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > > return; > > > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > > > - temp |= FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > > + temp &= ~FDI_BC_BIFURCATION_SELECT; > > + if (enable) > > + temp |= FDI_BC_BIFURCATION_SELECT; > > + > > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > > I915_WRITE(SOUTH_CHICKEN1, temp); > > POSTING_READ(SOUTH_CHICKEN1); > > } > > > > -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > > + bool pipe_active) > > { > > struct drm_device *dev = intel_crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > > > switch (intel_crtc->pipe) { > > case PIPE_A: > > break; > > case PIPE_B: > > - if (intel_crtc->config->fdi_lanes > 2) > > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > > + if (intel_crtc->config->fdi_lanes > 2 && pipe_active) > > + cpt_enable_fdi_bc_bifurcation(dev, false); > > else > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_enable_fdi_bc_bifurcation(dev, true); > > > > break; > > case PIPE_C: > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_enable_fdi_bc_bifurcation(dev, pipe_active); > > > > It sees could now change the bifurcation while pipe B is active (but only > using two lanes). Not sure if the old code did that too, or if it might > cause problems. It shouldn't be possible and it'll anger the hw. cpt_enable_fdi_bc_bifurcation has WARN_ONs to make sure the fdi rx for both pch transcoder B & C are off to make sure we don't get this wrong. > Also depending on the order you disable the pipes you might end up with > bifurcation enabled or disabled. > > It seems to me that the simplest solution should be to have bifurcation > enabled at all times, except when pipe B really needs the four lanes. > Then the hardware state would only need to be changed when > enabling/disabling pipe B with four lanes. Rest of the time > crtc->enabled and fdi_lanes should be able to tell us which > configuration is possible and which not. Or am I missing something? > > Well, eventually we want to tie this into the atomic state so that we > can actaully have pipe B drop into two lane mode if pipe C needs the > lanes (and pipe B can still operate with two lanes). But I guess that's > still not achievable with the current code. Checking fdi lane constraints is already done in the compute_config code. And it assumes that it can always get the max, i.e. if pipe B is only using 2 lanes it'll just enable pipe C. Which implies that we indeed have to aggressive enable the bifurcate bit (since atm we don't support a modeset on pipe B). For atomic modeset we'd just need to extend that code to work with all pipes (like all the other compute_config code). That shouldn't be a lot fo trouble (since we can always throw the crtc for the other pipe into the update in atomic_check functions). -Daniel -- 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] 27+ messages in thread
* [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-10 19:14 ` Daniel Vetter @ 2015-03-11 11:35 ` Ander Conselvan de Oliveira 2015-03-11 11:37 ` Conselvan De Oliveira, Ander ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Ander Conselvan de Oliveira @ 2015-03-11 11:35 UTC (permalink / raw) To: daniel; +Cc: Ander Conselvan de Oliveira, intel-gfx Remove the global modeset resource function that would disable the bifurcation bit, and instead enable/disable it when enabling the pch transcoder. The mode set consistency check should prevent us from disabling the bit if pipe C is enabled so the change should be safe. Note that this doens't affect the logic that prevents the bit being set while a pipe is active, since the patch retains the behavior of only chaging the bit if necessary. Because of the checks during mode set, the first change would necessarily happen with both pipes B and C disabled, and any subsequent write would be skipped. v2: Only change the bit during pch trancoder enable. (Ville) --- drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- 1 file changed, 10 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4008bf4..bfbd829 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) return crtc->base.state->enable && crtc->config->has_pch_encoder; } -static void ivb_modeset_global_resources(struct drm_device *dev) -{ - 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]); - struct intel_crtc *pipe_C_crtc = - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); - uint32_t temp; - - /* - * When everything is off disable fdi C so that we could enable fdi B - * with all lanes. Note that we don't care about enabled pipes without - * an enabled pch encoder. - */ - if (!pipe_has_enabled_pch(pipe_B_crtc) && - !pipe_has_enabled_pch(pipe_C_crtc)) { - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); - - temp = I915_READ(SOUTH_CHICKEN1); - temp &= ~FDI_BC_BIFURCATION_SELECT; - DRM_DEBUG_KMS("disabling fdi C rx\n"); - I915_WRITE(SOUTH_CHICKEN1, temp); - } -} - /* The FDI link training functions for ILK/Ibexpeak. */ static void ironlake_fdi_link_train(struct drm_crtc *crtc) { @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, I915_READ(VSYNCSHIFT(cpu_transcoder))); } -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) { struct drm_i915_private *dev_priv = dev->dev_private; uint32_t temp; temp = I915_READ(SOUTH_CHICKEN1); - if (temp & FDI_BC_BIFURCATION_SELECT) + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) return; WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); - temp |= FDI_BC_BIFURCATION_SELECT; - DRM_DEBUG_KMS("enabling fdi C rx\n"); + temp &= ~FDI_BC_BIFURCATION_SELECT; + if (enable) + temp |= FDI_BC_BIFURCATION_SELECT; + + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); I915_WRITE(SOUTH_CHICKEN1, temp); POSTING_READ(SOUTH_CHICKEN1); } @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) { struct drm_device *dev = intel_crtc->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; switch (intel_crtc->pipe) { case PIPE_A: break; case PIPE_B: if (intel_crtc->config->fdi_lanes > 2) - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); + cpt_set_fdi_bc_bifurcation(dev, false); else - cpt_enable_fdi_bc_bifurcation(dev); + cpt_set_fdi_bc_bifurcation(dev, true); break; case PIPE_C: - cpt_enable_fdi_bc_bifurcation(dev); + cpt_set_fdi_bc_bifurcation(dev, true); break; default: @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) } else if (IS_IVYBRIDGE(dev)) { /* FIXME: detect B0+ stepping and use auto training */ dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; - dev_priv->display.modeset_global_resources = - ivb_modeset_global_resources; } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { dev_priv->display.fdi_link_train = hsw_fdi_link_train; } else if (IS_VALLEYVIEW(dev)) { -- 2.1.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 11:35 ` Ander Conselvan de Oliveira @ 2015-03-11 11:37 ` Conselvan De Oliveira, Ander 2015-03-11 16:58 ` Ville Syrjälä 2015-03-11 12:24 ` Ville Syrjälä ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Conselvan De Oliveira, Ander @ 2015-03-11 11:37 UTC (permalink / raw) To: daniel; +Cc: intel-gfx On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote: > Remove the global modeset resource function that would disable the > bifurcation bit, and instead enable/disable it when enabling the pch > transcoder. The mode set consistency check should prevent us from > disabling the bit if pipe C is enabled so the change should be safe. > > Note that this doens't affect the logic that prevents the bit being > set while a pipe is active, since the patch retains the behavior of > only chaging the bit if necessary. Because of the checks during mode > set, the first change would necessarily happen with both pipes B and > C disabled, and any subsequent write would be skipped. > > v2: Only change the bit during pch trancoder enable. (Ville) Oops, I forgot the sob line. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- > 1 file changed, 10 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4008bf4..bfbd829 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > -static void ivb_modeset_global_resources(struct drm_device *dev) > -{ > - 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]); > - struct intel_crtc *pipe_C_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > - uint32_t temp; > - > - /* > - * When everything is off disable fdi C so that we could enable fdi B > - * with all lanes. Note that we don't care about enabled pipes without > - * an enabled pch encoder. > - */ > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > - !pipe_has_enabled_pch(pipe_C_crtc)) { > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > - > - temp = I915_READ(SOUTH_CHICKEN1); > - temp &= ~FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > - I915_WRITE(SOUTH_CHICKEN1, temp); > - } > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > I915_READ(VSYNCSHIFT(cpu_transcoder))); > } > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t temp; > > temp = I915_READ(SOUTH_CHICKEN1); > - if (temp & FDI_BC_BIFURCATION_SELECT) > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > return; > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - temp |= FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > + temp &= ~FDI_BC_BIFURCATION_SELECT; > + if (enable) > + temp |= FDI_BC_BIFURCATION_SELECT; > + > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > I915_WRITE(SOUTH_CHICKEN1, temp); > POSTING_READ(SOUTH_CHICKEN1); > } > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > { > struct drm_device *dev = intel_crtc->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > > switch (intel_crtc->pipe) { > case PIPE_A: > break; > case PIPE_B: > if (intel_crtc->config->fdi_lanes > 2) > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > + cpt_set_fdi_bc_bifurcation(dev, false); > else > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > case PIPE_C: > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > default: > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > - dev_priv->display.modeset_global_resources = > - ivb_modeset_global_resources; > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > } else if (IS_VALLEYVIEW(dev)) { --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 11:37 ` Conselvan De Oliveira, Ander @ 2015-03-11 16:58 ` Ville Syrjälä 2015-03-11 20:42 ` Daniel Vetter 0 siblings, 1 reply; 27+ messages in thread From: Ville Syrjälä @ 2015-03-11 16:58 UTC (permalink / raw) To: Conselvan De Oliveira, Ander; +Cc: intel-gfx On Wed, Mar 11, 2015 at 11:37:54AM +0000, Conselvan De Oliveira, Ander wrote: > On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote: > > Remove the global modeset resource function that would disable the > > bifurcation bit, and instead enable/disable it when enabling the pch > > transcoder. The mode set consistency check should prevent us from > > disabling the bit if pipe C is enabled so the change should be safe. > > > > Note that this doens't affect the logic that prevents the bit being > > set while a pipe is active, since the patch retains the behavior of > > only chaging the bit if necessary. Because of the checks during mode > > set, the first change would necessarily happen with both pipes B and > > C disabled, and any subsequent write would be skipped. > > > > v2: Only change the bit during pch trancoder enable. (Ville) > > Oops, I forgot the sob line. > > Signed-off-by: Ander Conselvan de Oliveira > <ander.conselvan.de.oliveira@intel.com> So I was staring at this stuff for a while and I believe it should be fine. We don't keep the bifurcation state entirely consistent when neither of the the pipes B/C are actually driving a PCH transcoder, but that shouldn't really matter. If we want to make it consistent then I suggest that we go with my earlier idea of only changing the state at transcoder B with >2 lanes enable/disable, and otherwise keep it enabled all the time. The slight complication there is the initial state we get from the BIOS which might not match that, so we'd need to sanitize it or something. Anyway, I also posted a couple of patches on top that try to sort out ironlake_check_fdi_lanes() [1]. With those and this one I think things should work even better than before. So for this patch: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061828.html > > > --- > > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- > > 1 file changed, 10 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 4008bf4..bfbd829 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > > return crtc->base.state->enable && crtc->config->has_pch_encoder; > > } > > > > -static void ivb_modeset_global_resources(struct drm_device *dev) > > -{ > > - 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]); > > - struct intel_crtc *pipe_C_crtc = > > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > > - uint32_t temp; > > - > > - /* > > - * When everything is off disable fdi C so that we could enable fdi B > > - * with all lanes. Note that we don't care about enabled pipes without > > - * an enabled pch encoder. > > - */ > > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > > - !pipe_has_enabled_pch(pipe_C_crtc)) { > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - > > - temp = I915_READ(SOUTH_CHICKEN1); > > - temp &= ~FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > > - I915_WRITE(SOUTH_CHICKEN1, temp); > > - } > > -} > > - > > /* The FDI link training functions for ILK/Ibexpeak. */ > > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > > { > > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > > I915_READ(VSYNCSHIFT(cpu_transcoder))); > > } > > > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > uint32_t temp; > > > > temp = I915_READ(SOUTH_CHICKEN1); > > - if (temp & FDI_BC_BIFURCATION_SELECT) > > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > > return; > > > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > > > - temp |= FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > > + temp &= ~FDI_BC_BIFURCATION_SELECT; > > + if (enable) > > + temp |= FDI_BC_BIFURCATION_SELECT; > > + > > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > > I915_WRITE(SOUTH_CHICKEN1, temp); > > POSTING_READ(SOUTH_CHICKEN1); > > } > > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > > static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > > { > > struct drm_device *dev = intel_crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > > > switch (intel_crtc->pipe) { > > case PIPE_A: > > break; > > case PIPE_B: > > if (intel_crtc->config->fdi_lanes > 2) > > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > > + cpt_set_fdi_bc_bifurcation(dev, false); > > else > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_set_fdi_bc_bifurcation(dev, true); > > > > break; > > case PIPE_C: > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_set_fdi_bc_bifurcation(dev, true); > > > > break; > > default: > > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) > > } else if (IS_IVYBRIDGE(dev)) { > > /* FIXME: detect B0+ stepping and use auto training */ > > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > > - dev_priv->display.modeset_global_resources = > > - ivb_modeset_global_resources; > > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > > } else if (IS_VALLEYVIEW(dev)) { > -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 16:58 ` Ville Syrjälä @ 2015-03-11 20:42 ` Daniel Vetter 0 siblings, 0 replies; 27+ messages in thread From: Daniel Vetter @ 2015-03-11 20:42 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Conselvan De Oliveira, Ander, intel-gfx On Wed, Mar 11, 2015 at 06:58:12PM +0200, Ville Syrjälä wrote: > On Wed, Mar 11, 2015 at 11:37:54AM +0000, Conselvan De Oliveira, Ander wrote: > > On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote: > > > Remove the global modeset resource function that would disable the > > > bifurcation bit, and instead enable/disable it when enabling the pch > > > transcoder. The mode set consistency check should prevent us from > > > disabling the bit if pipe C is enabled so the change should be safe. > > > > > > Note that this doens't affect the logic that prevents the bit being > > > set while a pipe is active, since the patch retains the behavior of > > > only chaging the bit if necessary. Because of the checks during mode > > > set, the first change would necessarily happen with both pipes B and > > > C disabled, and any subsequent write would be skipped. > > > > > > v2: Only change the bit during pch trancoder enable. (Ville) > > > > Oops, I forgot the sob line. > > > > Signed-off-by: Ander Conselvan de Oliveira > > <ander.conselvan.de.oliveira@intel.com> > > > So I was staring at this stuff for a while and I believe it should be > fine. We don't keep the bifurcation state entirely consistent when > neither of the the pipes B/C are actually driving a PCH transcoder, but > that shouldn't really matter. If we want to make it consistent then I > suggest that we go with my earlier idea of only changing the state at > transcoder B with >2 lanes enable/disable, and otherwise keep it enabled > all the time. The slight complication there is the initial state we get > from the BIOS which might not match that, so we'd need to sanitize it > or something. > > Anyway, I also posted a couple of patches on top that try to sort out > ironlake_check_fdi_lanes() [1]. With those and this one I think things > should work even better than before. > > So for this patch: > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Queued for -next, thanks for the patch. -Daniel >-- 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] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 11:35 ` Ander Conselvan de Oliveira 2015-03-11 11:37 ` Conselvan De Oliveira, Ander @ 2015-03-11 12:24 ` Ville Syrjälä 2015-03-11 13:10 ` Ville Syrjälä 2015-03-11 20:12 ` shuang.he 3 siblings, 0 replies; 27+ messages in thread From: Ville Syrjälä @ 2015-03-11 12:24 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Wed, Mar 11, 2015 at 01:35:43PM +0200, Ander Conselvan de Oliveira wrote: > Remove the global modeset resource function that would disable the > bifurcation bit, and instead enable/disable it when enabling the pch > transcoder. The mode set consistency check should prevent us from > disabling the bit if pipe C is enabled so the change should be safe. > > Note that this doens't affect the logic that prevents the bit being > set while a pipe is active, since the patch retains the behavior of > only chaging the bit if necessary. Because of the checks during mode > set, the first change would necessarily happen with both pipes B and > C disabled, and any subsequent write would be skipped. > > v2: Only change the bit during pch trancoder enable. (Ville) Actually what I had inb mind was something like this: pch_enable() { if (pipe == B && fdi_lanes > 2) disable_bifurcation() ... } pch_disable() { ... if (pipe == B && fdi_lanes > 2) enable_bifurcation() } So we only change it when enabling/disabling pipe B, never for pipe C. Hence it remains enabled whenever pipe B doesn't need >2 FDI lanes. > --- > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- > 1 file changed, 10 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4008bf4..bfbd829 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > -static void ivb_modeset_global_resources(struct drm_device *dev) > -{ > - 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]); > - struct intel_crtc *pipe_C_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > - uint32_t temp; > - > - /* > - * When everything is off disable fdi C so that we could enable fdi B > - * with all lanes. Note that we don't care about enabled pipes without > - * an enabled pch encoder. > - */ > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > - !pipe_has_enabled_pch(pipe_C_crtc)) { > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > - > - temp = I915_READ(SOUTH_CHICKEN1); > - temp &= ~FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > - I915_WRITE(SOUTH_CHICKEN1, temp); > - } > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > I915_READ(VSYNCSHIFT(cpu_transcoder))); > } > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t temp; > > temp = I915_READ(SOUTH_CHICKEN1); > - if (temp & FDI_BC_BIFURCATION_SELECT) > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > return; > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - temp |= FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > + temp &= ~FDI_BC_BIFURCATION_SELECT; > + if (enable) > + temp |= FDI_BC_BIFURCATION_SELECT; > + > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > I915_WRITE(SOUTH_CHICKEN1, temp); > POSTING_READ(SOUTH_CHICKEN1); > } > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > { > struct drm_device *dev = intel_crtc->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > > switch (intel_crtc->pipe) { > case PIPE_A: > break; > case PIPE_B: > if (intel_crtc->config->fdi_lanes > 2) > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > + cpt_set_fdi_bc_bifurcation(dev, false); > else > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > case PIPE_C: > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > default: > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > - dev_priv->display.modeset_global_resources = > - ivb_modeset_global_resources; > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > } else if (IS_VALLEYVIEW(dev)) { > -- > 2.1.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 11:35 ` Ander Conselvan de Oliveira 2015-03-11 11:37 ` Conselvan De Oliveira, Ander 2015-03-11 12:24 ` Ville Syrjälä @ 2015-03-11 13:10 ` Ville Syrjälä 2015-03-11 13:23 ` Conselvan De Oliveira, Ander 2015-03-11 20:12 ` shuang.he 3 siblings, 1 reply; 27+ messages in thread From: Ville Syrjälä @ 2015-03-11 13:10 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Wed, Mar 11, 2015 at 01:35:43PM +0200, Ander Conselvan de Oliveira wrote: > Remove the global modeset resource function that would disable the > bifurcation bit, and instead enable/disable it when enabling the pch > transcoder. The mode set consistency check should prevent us from > disabling the bit if pipe C is enabled so the change should be safe. > > Note that this doens't affect the logic that prevents the bit being > set while a pipe is active, since the patch retains the behavior of > only chaging the bit if necessary. Because of the checks during mode > set, the first change would necessarily happen with both pipes B and > C disabled, and any subsequent write would be skipped. > > v2: Only change the bit during pch trancoder enable. (Ville) > --- > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- > 1 file changed, 10 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4008bf4..bfbd829 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > -static void ivb_modeset_global_resources(struct drm_device *dev) > -{ > - 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]); > - struct intel_crtc *pipe_C_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > - uint32_t temp; > - > - /* > - * When everything is off disable fdi C so that we could enable fdi B > - * with all lanes. Note that we don't care about enabled pipes without > - * an enabled pch encoder. > - */ > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > - !pipe_has_enabled_pch(pipe_C_crtc)) { > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > - > - temp = I915_READ(SOUTH_CHICKEN1); > - temp &= ~FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > - I915_WRITE(SOUTH_CHICKEN1, temp); > - } > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > I915_READ(VSYNCSHIFT(cpu_transcoder))); > } > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t temp; > > temp = I915_READ(SOUTH_CHICKEN1); > - if (temp & FDI_BC_BIFURCATION_SELECT) > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > return; > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - temp |= FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > + temp &= ~FDI_BC_BIFURCATION_SELECT; > + if (enable) > + temp |= FDI_BC_BIFURCATION_SELECT; > + > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > I915_WRITE(SOUTH_CHICKEN1, temp); > POSTING_READ(SOUTH_CHICKEN1); > } > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > { > struct drm_device *dev = intel_crtc->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > > switch (intel_crtc->pipe) { > case PIPE_A: > break; > case PIPE_B: > if (intel_crtc->config->fdi_lanes > 2) > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > + cpt_set_fdi_bc_bifurcation(dev, false); So we just replace the globla_resources enforced assumed state with an explicit state change here. Should be perfectly fine since pipe is not active at this point. What really confuses me about the whole FDI bifurcation code is ironlake_check_fdi_lanes(). First of all I would rewrite it a bit like so: --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5615,14 +5615,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; } And the next thing confusing me is why we use pipe_has_enabled_pch() to check pipe B state, but just pipe->enabled to check pipe C state? Consider the following scenario: 1. enable pipe B with PCH using >2 FDI lanes 2. set pipe B to DPMS off 3. enable pipe C with PCH The crtc->active check in pipe_has_enabled_pch() will indicate that pipe B does not need FDI, so step 3 will succeed. Or am I missing something subtle here? Also if we do things this way: 1. enable pipe C with eDP 2. enable pipe B with PCH using >2 FDI lanes Now step 2 will fail even though pipe C doesn't need any FDI lanes. So to fix that it would seem that we should remove the crtc->active check from pipe_has_enabled_pch() and use that to check for conlicts in both cases. Now that .global_resources() is no longer in the picture the crtc->active check not needed anyway, I think. Thoughts? > else > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > case PIPE_C: > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_set_fdi_bc_bifurcation(dev, true); > > break; > default: > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > - dev_priv->display.modeset_global_resources = > - ivb_modeset_global_resources; > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > } else if (IS_VALLEYVIEW(dev)) { > -- > 2.1.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 13:10 ` Ville Syrjälä @ 2015-03-11 13:23 ` Conselvan De Oliveira, Ander 0 siblings, 0 replies; 27+ messages in thread From: Conselvan De Oliveira, Ander @ 2015-03-11 13:23 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Wed, 2015-03-11 at 15:10 +0200, Ville Syrjälä wrote: > On Wed, Mar 11, 2015 at 01:35:43PM +0200, Ander Conselvan de Oliveira wrote: > > Remove the global modeset resource function that would disable the > > bifurcation bit, and instead enable/disable it when enabling the pch > > transcoder. The mode set consistency check should prevent us from > > disabling the bit if pipe C is enabled so the change should be safe. > > > > Note that this doens't affect the logic that prevents the bit being > > set while a pipe is active, since the patch retains the behavior of > > only chaging the bit if necessary. Because of the checks during mode > > set, the first change would necessarily happen with both pipes B and > > C disabled, and any subsequent write would be skipped. > > > > v2: Only change the bit during pch trancoder enable. (Ville) > > --- > > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++---------------------------- > > 1 file changed, 10 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 4008bf4..bfbd829 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > > return crtc->base.state->enable && crtc->config->has_pch_encoder; > > } > > > > -static void ivb_modeset_global_resources(struct drm_device *dev) > > -{ > > - 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]); > > - struct intel_crtc *pipe_C_crtc = > > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > > - uint32_t temp; > > - > > - /* > > - * When everything is off disable fdi C so that we could enable fdi B > > - * with all lanes. Note that we don't care about enabled pipes without > > - * an enabled pch encoder. > > - */ > > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > > - !pipe_has_enabled_pch(pipe_C_crtc)) { > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - > > - temp = I915_READ(SOUTH_CHICKEN1); > > - temp &= ~FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > > - I915_WRITE(SOUTH_CHICKEN1, temp); > > - } > > -} > > - > > /* The FDI link training functions for ILK/Ibexpeak. */ > > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > > { > > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > > I915_READ(VSYNCSHIFT(cpu_transcoder))); > > } > > > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > uint32_t temp; > > > > temp = I915_READ(SOUTH_CHICKEN1); > > - if (temp & FDI_BC_BIFURCATION_SELECT) > > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > > return; > > > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > > > - temp |= FDI_BC_BIFURCATION_SELECT; > > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > > + temp &= ~FDI_BC_BIFURCATION_SELECT; > > + if (enable) > > + temp |= FDI_BC_BIFURCATION_SELECT; > > + > > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > > I915_WRITE(SOUTH_CHICKEN1, temp); > > POSTING_READ(SOUTH_CHICKEN1); > > } > > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > > static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > > { > > struct drm_device *dev = intel_crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > > > switch (intel_crtc->pipe) { > > case PIPE_A: > > break; > > case PIPE_B: > > if (intel_crtc->config->fdi_lanes > 2) > > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > > + cpt_set_fdi_bc_bifurcation(dev, false); > > So we just replace the globla_resources enforced assumed state with an > explicit state change here. Should be perfectly fine since pipe is not > active at this point. > > > What really confuses me about the whole FDI bifurcation code is > ironlake_check_fdi_lanes(). First of all I would rewrite it a bit like > so: > > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5615,14 +5615,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; > } > > And the next thing confusing me is why we use pipe_has_enabled_pch() > to check pipe B state, but just pipe->enabled to check pipe C state? > > Consider the following scenario: > 1. enable pipe B with PCH using >2 FDI lanes > 2. set pipe B to DPMS off > 3. enable pipe C with PCH > > The crtc->active check in pipe_has_enabled_pch() will indicate that pipe > B does not need FDI, so step 3 will succeed. Or am I missing something > subtle here? > > Also if we do things this way: > 1. enable pipe C with eDP > 2. enable pipe B with PCH using >2 FDI lanes > > Now step 2 will fail even though pipe C doesn't need any FDI lanes. > > So to fix that it would seem that we should remove the crtc->active > check from pipe_has_enabled_pch() and use that to check for conlicts > in both cases. Now that .global_resources() is no longer in the picture > the crtc->active check not needed anyway, I think. This is actually the motivation for this patch. I tried to fix that in http://patchwork.freedesktop.org/patch/44281/ , but then it became clear that patch broke the case of pipe B going from a >2 lanes mode to a <=2 mode. But the combination of the two patches works for both cases. I haven't thought about the eDP on pipe C case before, but it seems we want the change you mentioned above too to fix that. Ander > > > else > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_set_fdi_bc_bifurcation(dev, true); > > > > break; > > case PIPE_C: > > - cpt_enable_fdi_bc_bifurcation(dev); > > + cpt_set_fdi_bc_bifurcation(dev, true); > > > > break; > > default: > > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev) > > } else if (IS_IVYBRIDGE(dev)) { > > /* FIXME: detect B0+ stepping and use auto training */ > > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > > - dev_priv->display.modeset_global_resources = > > - ivb_modeset_global_resources; > > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > > } else if (IS_VALLEYVIEW(dev)) { > > -- > > 2.1.0 > --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-11 11:35 ` Ander Conselvan de Oliveira ` (2 preceding siblings ...) 2015-03-11 13:10 ` Ville Syrjälä @ 2015-03-11 20:12 ` shuang.he 3 siblings, 0 replies; 27+ messages in thread From: shuang.he @ 2015-03-11 20:12 UTC (permalink / raw) To: shuang.he, ethan.gao, intel-gfx, ander.conselvan.de.oliveira Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Task id: 5933 -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV -3 281/281 278/281 ILK 308/308 308/308 SNB -1 284/284 283/284 IVB 375/375 375/375 BYT 294/294 294/294 HSW 384/384 384/384 BDW 315/315 315/315 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(3)PASS(2) CRASH(1)PASS(1) PNV igt_gen3_render_tiledy_blits FAIL(3)PASS(1) FAIL(1)PASS(1) *PNV igt_gem_fence_thrash_bo-write-verify-threaded-none PASS(4) CRASH(1)PASS(1) *SNB igt_gem_exec_params_sol-reset-not-gen7 PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira 2015-03-10 12:35 ` [PATCH] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira 2015-03-10 13:03 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ville Syrjälä @ 2015-03-10 19:10 ` Daniel Vetter 2015-03-10 20:40 ` shuang.he 3 siblings, 0 replies; 27+ messages in thread From: Daniel Vetter @ 2015-03-10 19:10 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Tue, Mar 10, 2015 at 02:32:34PM +0200, Ander Conselvan de Oliveira wrote: > Remove the global modeset resource function that would disable the > bifurcation bit, and instead enable/disable it when enabling or > disabling the pch transcoder. The checks before the mode set should > ensure that the configuration is valid, so it should be safe to do it > so. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > > On Mon, 2015-03-09 at 17:21 +0100, Daniel Vetter wrote: > > With atomic we need to probably look at crtc_state->mode_changed. As an > > interim solution we have the same information available in the > > modeset_pipes bitmask. I think replacing the check for intel_crtc->active > > with !(modeset_pipes & (1<<intel_crtc->pipe)) should work. > > I looked into that solution, but involves passing modeset_pipes way down > into the call stack, so I started looking for a different solution. Do you > see any caveat with this approach? Moving things to the disable hook would be great since that's yet another special case remove. It wasnt' possible back when I've done this, and I think the reason was that we still had a ->mode_set callback before the crtc_enable hook. But that's all gone now, so as long as the bifurcate update is done early enough I think this'll work. Maybe discuss this with Ville locally - he provided all the feedback for my original patch too? Random bikesheds below, I definitely need to think about this more. Cheers, Daniel > > Thanks, > Ander > > --- > drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++----------------------- > 1 file changed, 21 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4008bf4..eca5a41 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -101,6 +101,8 @@ static void chv_prepare_pll(struct intel_crtc *crtc, > const struct intel_crtc_state *pipe_config); > static void intel_begin_crtc_commit(struct drm_crtc *crtc); > static void intel_finish_crtc_commit(struct drm_crtc *crtc); > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > + bool pipe_active); Imo just move the functions instead of forward declarations. > > static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe) > { > @@ -1956,6 +1958,8 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > enum pipe pipe) > { > struct drm_device *dev = dev_priv->dev; > + struct intel_crtc *crtc = > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > uint32_t reg, val; > > /* FDI relies on the transcoder */ > @@ -1980,6 +1984,9 @@ static void ironlake_disable_pch_transcoder(struct drm_i915_private *dev_priv, > val &= ~TRANS_CHICKEN2_TIMING_OVERRIDE; > I915_WRITE(reg, val); > } > + > + if (IS_IVYBRIDGE(dev)) > + ivybridge_update_fdi_bc_bifurcation(crtc, false); > } > > static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv) > @@ -3153,32 +3160,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc) > return crtc->base.state->enable && crtc->config->has_pch_encoder; > } > > -static void ivb_modeset_global_resources(struct drm_device *dev) > -{ > - 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]); > - struct intel_crtc *pipe_C_crtc = > - to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); > - uint32_t temp; > - > - /* > - * When everything is off disable fdi C so that we could enable fdi B > - * with all lanes. Note that we don't care about enabled pipes without > - * an enabled pch encoder. > - */ > - if (!pipe_has_enabled_pch(pipe_B_crtc) && > - !pipe_has_enabled_pch(pipe_C_crtc)) { > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > - > - temp = I915_READ(SOUTH_CHICKEN1); > - temp &= ~FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("disabling fdi C rx\n"); > - I915_WRITE(SOUTH_CHICKEN1, temp); > - } > -} > - > /* The FDI link training functions for ILK/Ibexpeak. */ > static void ironlake_fdi_link_train(struct drm_crtc *crtc) > { > @@ -3834,41 +3815,44 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, > I915_READ(VSYNCSHIFT(cpu_transcoder))); > } > > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) > +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev, bool enable) enable feels misnamed now. Maybe s/enable/set/ instead? > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t temp; > > temp = I915_READ(SOUTH_CHICKEN1); > - if (temp & FDI_BC_BIFURCATION_SELECT) > + if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable) > return; > > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); > WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); > > - temp |= FDI_BC_BIFURCATION_SELECT; > - DRM_DEBUG_KMS("enabling fdi C rx\n"); > + temp &= ~FDI_BC_BIFURCATION_SELECT; > + if (enable) > + temp |= FDI_BC_BIFURCATION_SELECT; > + > + DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis"); > I915_WRITE(SOUTH_CHICKEN1, temp); > POSTING_READ(SOUTH_CHICKEN1); > } > > -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) > +static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc, > + bool pipe_active) > { > struct drm_device *dev = intel_crtc->base.dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > > switch (intel_crtc->pipe) { > case PIPE_A: > break; > case PIPE_B: > - if (intel_crtc->config->fdi_lanes > 2) > - WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); > + if (intel_crtc->config->fdi_lanes > 2 && pipe_active) > + cpt_enable_fdi_bc_bifurcation(dev, false); > else > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_enable_fdi_bc_bifurcation(dev, true); > > break; > case PIPE_C: > - cpt_enable_fdi_bc_bifurcation(dev); > + cpt_enable_fdi_bc_bifurcation(dev, pipe_active); > > break; > default: > @@ -3895,7 +3879,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc) > assert_pch_transcoder_disabled(dev_priv, pipe); > > if (IS_IVYBRIDGE(dev)) > - ivybridge_update_fdi_bc_bifurcation(intel_crtc); > + ivybridge_update_fdi_bc_bifurcation(intel_crtc, true); > > /* Write the TU size bits before fdi link training, so that error > * detection works. */ > @@ -13056,8 +13040,6 @@ static void intel_init_display(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev)) { > /* FIXME: detect B0+ stepping and use auto training */ > dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; > - dev_priv->display.modeset_global_resources = > - ivb_modeset_global_resources; > } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { > dev_priv->display.fdi_link_train = hsw_fdi_link_train; > } else if (IS_VALLEYVIEW(dev)) { > -- > 2.1.0 > -- 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] 27+ messages in thread
* Re: [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira ` (2 preceding siblings ...) 2015-03-10 19:10 ` Daniel Vetter @ 2015-03-10 20:40 ` shuang.he 3 siblings, 0 replies; 27+ messages in thread From: shuang.he @ 2015-03-10 20:40 UTC (permalink / raw) To: shuang.he, ethan.gao, intel-gfx, ander.conselvan.de.oliveira Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Task id: 5925 -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV -2 282/282 280/282 ILK 308/308 308/308 SNB 307/307 307/307 IVB 375/375 375/375 BYT 294/294 294/294 HSW 385/385 385/385 BDW 315/315 315/315 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied PNV igt_gem_userptr_blits_minor-normal-sync DMESG_WARN(2)PASS(3) DMESG_WARN(2) *PNV igt_gem_userptr_blits_minor-unsync-normal PASS(4) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C 2015-03-09 8:59 [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C Ander Conselvan de Oliveira 2015-03-09 9:24 ` Jani Nikula @ 2015-03-09 12:17 ` shuang.he 1 sibling, 0 replies; 27+ messages in thread From: shuang.he @ 2015-03-09 12:17 UTC (permalink / raw) To: shuang.he, ethan.gao, intel-gfx, ander.conselvan.de.oliveira Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) Task id: 5915 -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV -1 282/282 281/282 ILK 308/308 308/308 SNB 307/307 307/307 IVB -2 375/375 373/375 BYT 294/294 294/294 HSW 385/385 385/385 BDW 315/315 315/315 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *PNV igt_gem_userptr_blits_minor-unsync-normal PASS(2) DMESG_WARN(1)PASS(1) *IVB igt_drv_debugfs_reader PASS(2) DMESG_WARN(2) *IVB igt_drv_hangman_error-state-sysfs-entry PASS(2) TIMEOUT(2) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2015-03-27 13:36 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-09 8:59 [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C Ander Conselvan de Oliveira 2015-03-09 9:24 ` Jani Nikula 2015-03-09 9:33 ` Ander Conselvan De Oliveira 2015-03-09 16:21 ` Daniel Vetter 2015-03-10 12:32 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ander Conselvan de Oliveira 2015-03-10 12:35 ` [PATCH] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira 2015-03-10 19:05 ` Daniel Vetter 2015-03-11 11:33 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Ander Conselvan de Oliveira 2015-03-11 11:33 ` [PATCH igt 2/2] tests: Add test for pipe B and C interactions in IVB Ander Conselvan de Oliveira 2015-03-27 13:35 ` Thomas Wood 2015-03-11 13:26 ` [PATCH igt 1/2] lib/kms: Add a way to override an output's mode Damien Lespiau 2015-03-11 13:48 ` Ander Conselvan De Oliveira 2015-03-11 14:26 ` Damien Lespiau 2015-03-27 13:30 ` Thomas Wood 2015-03-10 13:03 ` [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept Ville Syrjälä 2015-03-10 19:14 ` Daniel Vetter 2015-03-11 11:35 ` Ander Conselvan de Oliveira 2015-03-11 11:37 ` Conselvan De Oliveira, Ander 2015-03-11 16:58 ` Ville Syrjälä 2015-03-11 20:42 ` Daniel Vetter 2015-03-11 12:24 ` Ville Syrjälä 2015-03-11 13:10 ` Ville Syrjälä 2015-03-11 13:23 ` Conselvan De Oliveira, Ander 2015-03-11 20:12 ` shuang.he 2015-03-10 19:10 ` Daniel Vetter 2015-03-10 20:40 ` shuang.he 2015-03-09 12:17 ` [PATCH] drm/i915: Ignore pipe B active state when enabling pipe C shuang.he
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.