Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
On Wed, Mar 11, 2015 at 11:37:54AM +, 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.olive...@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.syrj...@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
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
On Wed, Mar 11, 2015 at 06:58:12PM +0200, Ville Syrjälä wrote: On Wed, Mar 11, 2015 at 11:37:54AM +, 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.olive...@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.syrj...@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
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@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 Testdrm-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
[Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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.olive...@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 =
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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);
-
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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,
-
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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 */
[Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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.olive...@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 (1intel_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
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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.olive...@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 (1intel_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);
}
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@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 Testdrm-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
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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.olive...@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 (1intel_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 :
Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept
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.olive...@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 (1intel_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;
