* [PATCH 2/2] drm/i915: drop a few really redundant WARNs in hsw mode_set
2013-05-28 14:00 [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Daniel Vetter
@ 2013-05-28 14:00 ` Daniel Vetter
2013-05-28 14:28 ` [PATCH] " Daniel Vetter
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-05-28 14:00 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
- Correct cpu->pch display matching is already check when we detect
the PCH type at driver load.
- Plane/pipe state is already checked both when a) enabling, b)
disabling and in c) the modeset state checker. No need to go
overboard and also check it in in between a) and b).
Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e9aa2a5..1216fdf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5861,15 +5861,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
int plane = intel_crtc->plane;
int ret;
- /* We are not sure yet this won't happen. */
- WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
- INTEL_PCH_TYPE(dev));
-
- WARN_ON(I915_READ(PIPECONF(intel_crtc->config.cpu_transcoder)) &
- (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
-
- WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
-
if (!intel_ddi_pll_mode_set(crtc))
return -EINVAL;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] drm/i915: drop a few really redundant WARNs in hsw mode_set
2013-05-28 14:00 [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Daniel Vetter
2013-05-28 14:00 ` [PATCH 2/2] drm/i915: drop a few really redundant WARNs in hsw mode_set Daniel Vetter
@ 2013-05-28 14:28 ` Daniel Vetter
2013-05-28 14:30 ` Paulo Zanoni
2013-05-29 15:44 ` [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Chris Wilson
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2013-05-28 14:28 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
- Correct cpu->pch display matching is already check when we detect
the PCH type at driver load.
- Plane/pipe state is already checked both when a) enabling, b)
disabling and in c) the modeset state checker. No need to go
overboard and also check it in in between a) and b).
Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5baf14c..a40c186 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5861,15 +5861,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
int plane = intel_crtc->plane;
int ret;
- /* We are not sure yet this won't happen. */
- WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
- INTEL_PCH_TYPE(dev));
-
- WARN_ON(I915_READ(PIPECONF(intel_crtc->config.cpu_transcoder)) &
- (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
-
- WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
-
if (!intel_ddi_pll_mode_set(crtc))
return -EINVAL;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] drm/i915: drop a few really redundant WARNs in hsw mode_set
2013-05-28 14:28 ` [PATCH] " Daniel Vetter
@ 2013-05-28 14:30 ` Paulo Zanoni
2013-05-29 7:35 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Paulo Zanoni @ 2013-05-28 14:30 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
2013/5/28 Daniel Vetter <daniel.vetter@ffwll.ch>:
> - Correct cpu->pch display matching is already check when we detect
> the PCH type at driver load.
> - Plane/pipe state is already checked both when a) enabling, b)
> disabling and in c) the modeset state checker. No need to go
> overboard and also check it in in between a) and b).
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5baf14c..a40c186 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5861,15 +5861,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> int plane = intel_crtc->plane;
> int ret;
>
> - /* We are not sure yet this won't happen. */
> - WARN(!HAS_PCH_LPT(dev), "Unexpected PCH type %d\n",
> - INTEL_PCH_TYPE(dev));
> -
> - WARN_ON(I915_READ(PIPECONF(intel_crtc->config.cpu_transcoder)) &
> - (PIPECONF_ENABLE | I965_PIPECONF_ACTIVE));
> -
> - WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
> -
> if (!intel_ddi_pll_mode_set(crtc))
> return -EINVAL;
>
> --
> 1.7.11.7
>
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: drop a few really redundant WARNs in hsw mode_set
2013-05-28 14:30 ` Paulo Zanoni
@ 2013-05-29 7:35 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-05-29 7:35 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development
On Tue, May 28, 2013 at 11:30:44AM -0300, Paulo Zanoni wrote:
> 2013/5/28 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > - Correct cpu->pch display matching is already check when we detect
> > the PCH type at driver load.
> > - Plane/pipe state is already checked both when a) enabling, b)
> > disabling and in c) the modeset state checker. No need to go
> > overboard and also check it in in between a) and b).
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks
2013-05-28 14:00 [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Daniel Vetter
2013-05-28 14:00 ` [PATCH 2/2] drm/i915: drop a few really redundant WARNs in hsw mode_set Daniel Vetter
2013-05-28 14:28 ` [PATCH] " Daniel Vetter
@ 2013-05-29 15:44 ` Chris Wilson
2013-05-29 15:52 ` [PATCH] " Daniel Vetter
2013-05-30 13:04 ` Daniel Vetter
4 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2013-05-29 15:44 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development, stable
On Tue, May 28, 2013 at 04:00:18PM +0200, Daniel Vetter wrote:
> Only lvds/tv did actually check for cloning or not, but many more
> places should.
>
> Notices because my ivb tried to enable both cpu edp and vga on the
> first crtc - the resulting confusion between has_pch_encoder,
> has_dp_encoder but not actually being a pch dp encoder resulting in
> hilarity (hitting a BUG).
>
> We _really_ need an igt to random-walk our modeset space more
> exhaustively.
>
> I haven't figured out yet why exactly we see this configuration
> request from the setcrtc ioctl, but I can reliably reproduce it by
> unplugging a bunch of connectors and restarting X.
>
> Strangely restarting X again fixes things ...
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> - if (intel_encoder_check_is_cloned(&lvds_encoder->base))
> - return false;
> -
Ok, silly question: If you are removing the callers, why not remove the
function as well?
Otherwise, the testing for requesting multiple uncloneable encoders on a
CRTC looks sane. So finish the job and take my r-b.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm/i915: consolidate and tighten encoder cloning checks
2013-05-28 14:00 [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Daniel Vetter
` (2 preceding siblings ...)
2013-05-29 15:44 ` [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Chris Wilson
@ 2013-05-29 15:52 ` Daniel Vetter
2013-05-30 13:04 ` Daniel Vetter
4 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-05-29 15:52 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Chris Wilson, Daniel Vetter, stable
Only lvds/tv did actually check for cloning or not, but many more
places should.
Notices because my ivb tried to enable both cpu edp and vga on the
first crtc - the resulting confusion between has_pch_encoder,
has_dp_encoder but not actually being a pch dp encoder resulting in
hilarity (hitting a BUG).
We _really_ need an igt to random-walk our modeset space more
exhaustively.
I haven't figured out yet why exactly we see this configuration
request from the setcrtc ioctl, but I can reliably reproduce it by
unplugging a bunch of connectors and restarting X.
Strangely restarting X again fixes things ...
v2: Kill intel_encoder_check_is_cloned, spotted by Paulo.
v3: Drop the now unused pipe param.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 65 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_lvds.c | 3 --
drivers/gpu/drm/i915/intel_tv.c | 3 --
4 files changed, 25 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 14bb844..4cfce96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5843,27 +5843,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
int plane = intel_crtc->plane;
- int num_connectors = 0;
- bool is_cpu_edp = false;
- struct intel_encoder *encoder;
int ret;
- for_each_encoder_on_crtc(dev, crtc, encoder) {
- switch (encoder->type) {
- case INTEL_OUTPUT_EDP:
- if (enc_to_dig_port(&encoder->base)->port == PORT_A)
- is_cpu_edp = true;
- break;
- }
-
- num_connectors++;
- }
-
- WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
- num_connectors, pipe_name(pipe));
-
if (!intel_ddi_pll_mode_set(crtc))
return -EINVAL;
@@ -7523,28 +7505,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
.load_lut = intel_crtc_load_lut,
};
-bool intel_encoder_check_is_cloned(struct intel_encoder *encoder)
-{
- struct intel_encoder *other_encoder;
- struct drm_crtc *crtc = &encoder->new_crtc->base;
-
- if (WARN_ON(!crtc))
- return false;
-
- list_for_each_entry(other_encoder,
- &crtc->dev->mode_config.encoder_list,
- base.head) {
-
- if (&other_encoder->new_crtc->base != crtc ||
- encoder == other_encoder)
- continue;
- else
- return true;
- }
-
- return false;
-}
-
static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
struct drm_crtc *crtc)
{
@@ -7713,6 +7673,25 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
pipe_config->pch_pfit.size);
}
+static bool check_encoder_cloning(struct drm_crtc *crtc)
+{
+ int num_encoders = 0;
+ bool uncloneable_encoders = false;
+ struct intel_encoder *encoder;
+
+ list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
+ base.head) {
+ if (&encoder->new_crtc->base != crtc)
+ continue;
+
+ num_encoders++;
+ if (!encoder->cloneable)
+ uncloneable_encoders = true;
+ }
+
+ return !(num_encoders > 1 && uncloneable_encoders);
+}
+
static struct intel_crtc_config *
intel_modeset_pipe_config(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
@@ -7725,6 +7704,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
int plane_bpp, ret = -EINVAL;
bool retry = true;
+ if (!check_encoder_cloning(crtc)) {
+ DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+ return ERR_PTR(-EINVAL);
+ }
+
pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
if (!pipe_config)
return ERR_PTR(-ENOMEM);
@@ -8834,6 +8818,7 @@ static void intel_setup_outputs(struct drm_device *dev)
encoder->base.possible_crtcs = encoder->crtc_mask;
encoder->base.possible_clones =
intel_encoder_clones(encoder);
+ printk("possible encoder clones 0x%08x\n", encoder->base.possible_clones);
}
intel_init_pch_refclk(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3db4b14..a844a05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -627,7 +627,6 @@ extern void intel_crtc_load_lut(struct drm_crtc *crtc);
extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
extern void intel_encoder_destroy(struct drm_encoder *encoder);
extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode);
-extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
extern void intel_connector_dpms(struct drm_connector *, int mode);
extern bool intel_connector_get_hw_state(struct intel_connector *connector);
extern void intel_modeset_check_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6554860..10c3d56 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -264,9 +264,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
return false;
}
- if (intel_encoder_check_is_cloned(&lvds_encoder->base))
- return false;
-
if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) ==
LVDS_A3_POWER_UP)
lvds_bpp = 8*3;
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 7d11a5a..39debd8 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -914,9 +914,6 @@ intel_tv_compute_config(struct intel_encoder *encoder,
if (!tv_mode)
return false;
- if (intel_encoder_check_is_cloned(&intel_tv->base))
- return false;
-
pipe_config->adjusted_mode.clock = tv_mode->clock;
DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
pipe_config->pipe_bpp = 8*3;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] drm/i915: consolidate and tighten encoder cloning checks
2013-05-28 14:00 [PATCH 1/2] drm/i915: consolidate and tighten encoder cloning checks Daniel Vetter
` (3 preceding siblings ...)
2013-05-29 15:52 ` [PATCH] " Daniel Vetter
@ 2013-05-30 13:04 ` Daniel Vetter
2013-06-05 10:37 ` Daniel Vetter
4 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2013-05-30 13:04 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Chris Wilson, stable
Only lvds/tv did actually check for cloning or not, but many more
places should.
Notices because my ivb tried to enable both cpu edp and vga on the
first crtc - the resulting confusion between has_pch_encoder,
has_dp_encoder but not actually being a pch dp encoder resulting in
hilarity (hitting a BUG).
We _really_ need an igt to random-walk our modeset space more
exhaustively.
I haven't figured out yet why exactly we see this configuration
request from the setcrtc ioctl, but I can reliably reproduce it by
unplugging a bunch of connectors and restarting X.
Strangely restarting X again fixes things ...
v2: Kill intel_encoder_check_is_cloned, spotted by Paulo.
v3: Drop the now unused pipe param.
v4: Kill the stray printk Chris spotted.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_lvds.c | 3 --
drivers/gpu/drm/i915/intel_tv.c | 3 --
4 files changed, 24 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 14bb844..ba33a82 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5843,27 +5843,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
int plane = intel_crtc->plane;
- int num_connectors = 0;
- bool is_cpu_edp = false;
- struct intel_encoder *encoder;
int ret;
- for_each_encoder_on_crtc(dev, crtc, encoder) {
- switch (encoder->type) {
- case INTEL_OUTPUT_EDP:
- if (enc_to_dig_port(&encoder->base)->port == PORT_A)
- is_cpu_edp = true;
- break;
- }
-
- num_connectors++;
- }
-
- WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
- num_connectors, pipe_name(pipe));
-
if (!intel_ddi_pll_mode_set(crtc))
return -EINVAL;
@@ -7523,28 +7505,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
.load_lut = intel_crtc_load_lut,
};
-bool intel_encoder_check_is_cloned(struct intel_encoder *encoder)
-{
- struct intel_encoder *other_encoder;
- struct drm_crtc *crtc = &encoder->new_crtc->base;
-
- if (WARN_ON(!crtc))
- return false;
-
- list_for_each_entry(other_encoder,
- &crtc->dev->mode_config.encoder_list,
- base.head) {
-
- if (&other_encoder->new_crtc->base != crtc ||
- encoder == other_encoder)
- continue;
- else
- return true;
- }
-
- return false;
-}
-
static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
struct drm_crtc *crtc)
{
@@ -7713,6 +7673,25 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
pipe_config->pch_pfit.size);
}
+static bool check_encoder_cloning(struct drm_crtc *crtc)
+{
+ int num_encoders = 0;
+ bool uncloneable_encoders = false;
+ struct intel_encoder *encoder;
+
+ list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
+ base.head) {
+ if (&encoder->new_crtc->base != crtc)
+ continue;
+
+ num_encoders++;
+ if (!encoder->cloneable)
+ uncloneable_encoders = true;
+ }
+
+ return !(num_encoders > 1 && uncloneable_encoders);
+}
+
static struct intel_crtc_config *
intel_modeset_pipe_config(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
@@ -7725,6 +7704,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
int plane_bpp, ret = -EINVAL;
bool retry = true;
+ if (!check_encoder_cloning(crtc)) {
+ DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+ return ERR_PTR(-EINVAL);
+ }
+
pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
if (!pipe_config)
return ERR_PTR(-ENOMEM);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3db4b14..a844a05 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -627,7 +627,6 @@ extern void intel_crtc_load_lut(struct drm_crtc *crtc);
extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
extern void intel_encoder_destroy(struct drm_encoder *encoder);
extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode);
-extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
extern void intel_connector_dpms(struct drm_connector *, int mode);
extern bool intel_connector_get_hw_state(struct intel_connector *connector);
extern void intel_modeset_check_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6554860..10c3d56 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -264,9 +264,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
return false;
}
- if (intel_encoder_check_is_cloned(&lvds_encoder->base))
- return false;
-
if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) ==
LVDS_A3_POWER_UP)
lvds_bpp = 8*3;
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 7d11a5a..39debd8 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -914,9 +914,6 @@ intel_tv_compute_config(struct intel_encoder *encoder,
if (!tv_mode)
return false;
- if (intel_encoder_check_is_cloned(&intel_tv->base))
- return false;
-
pipe_config->adjusted_mode.clock = tv_mode->clock;
DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
pipe_config->pipe_bpp = 8*3;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] drm/i915: consolidate and tighten encoder cloning checks
2013-05-30 13:04 ` Daniel Vetter
@ 2013-06-05 10:37 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-06-05 10:37 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, Chris Wilson, stable
On Thu, May 30, 2013 at 03:04:25PM +0200, Daniel Vetter wrote:
> Only lvds/tv did actually check for cloning or not, but many more
> places should.
>
> Notices because my ivb tried to enable both cpu edp and vga on the
> first crtc - the resulting confusion between has_pch_encoder,
> has_dp_encoder but not actually being a pch dp encoder resulting in
> hilarity (hitting a BUG).
>
> We _really_ need an igt to random-walk our modeset space more
> exhaustively.
>
> I haven't figured out yet why exactly we see this configuration
> request from the setcrtc ioctl, but I can reliably reproduce it by
> unplugging a bunch of connectors and restarting X.
>
> Strangely restarting X again fixes things ...
>
> v2: Kill intel_encoder_check_is_cloned, spotted by Paulo.
>
> v3: Drop the now unused pipe param.
>
> v4: Kill the stray printk Chris spotted.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Patch merged to dinq (imo a bit late for -fixes) with Chris' r-b.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 64 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_lvds.c | 3 --
> drivers/gpu/drm/i915/intel_tv.c | 3 --
> 4 files changed, 24 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 14bb844..ba33a82 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5843,27 +5843,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> - int num_connectors = 0;
> - bool is_cpu_edp = false;
> - struct intel_encoder *encoder;
> int ret;
>
> - for_each_encoder_on_crtc(dev, crtc, encoder) {
> - switch (encoder->type) {
> - case INTEL_OUTPUT_EDP:
> - if (enc_to_dig_port(&encoder->base)->port == PORT_A)
> - is_cpu_edp = true;
> - break;
> - }
> -
> - num_connectors++;
> - }
> -
> - WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
> - num_connectors, pipe_name(pipe));
> -
> if (!intel_ddi_pll_mode_set(crtc))
> return -EINVAL;
>
> @@ -7523,28 +7505,6 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
> .load_lut = intel_crtc_load_lut,
> };
>
> -bool intel_encoder_check_is_cloned(struct intel_encoder *encoder)
> -{
> - struct intel_encoder *other_encoder;
> - struct drm_crtc *crtc = &encoder->new_crtc->base;
> -
> - if (WARN_ON(!crtc))
> - return false;
> -
> - list_for_each_entry(other_encoder,
> - &crtc->dev->mode_config.encoder_list,
> - base.head) {
> -
> - if (&other_encoder->new_crtc->base != crtc ||
> - encoder == other_encoder)
> - continue;
> - else
> - return true;
> - }
> -
> - return false;
> -}
> -
> static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
> struct drm_crtc *crtc)
> {
> @@ -7713,6 +7673,25 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> pipe_config->pch_pfit.size);
> }
>
> +static bool check_encoder_cloning(struct drm_crtc *crtc)
> +{
> + int num_encoders = 0;
> + bool uncloneable_encoders = false;
> + struct intel_encoder *encoder;
> +
> + list_for_each_entry(encoder, &crtc->dev->mode_config.encoder_list,
> + base.head) {
> + if (&encoder->new_crtc->base != crtc)
> + continue;
> +
> + num_encoders++;
> + if (!encoder->cloneable)
> + uncloneable_encoders = true;
> + }
> +
> + return !(num_encoders > 1 && uncloneable_encoders);
> +}
> +
> static struct intel_crtc_config *
> intel_modeset_pipe_config(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> @@ -7725,6 +7704,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> int plane_bpp, ret = -EINVAL;
> bool retry = true;
>
> + if (!check_encoder_cloning(crtc)) {
> + DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> if (!pipe_config)
> return ERR_PTR(-ENOMEM);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3db4b14..a844a05 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -627,7 +627,6 @@ extern void intel_crtc_load_lut(struct drm_crtc *crtc);
> extern void intel_crtc_update_dpms(struct drm_crtc *crtc);
> extern void intel_encoder_destroy(struct drm_encoder *encoder);
> extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode);
> -extern bool intel_encoder_check_is_cloned(struct intel_encoder *encoder);
> extern void intel_connector_dpms(struct drm_connector *, int mode);
> extern bool intel_connector_get_hw_state(struct intel_connector *connector);
> extern void intel_modeset_check_state(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 6554860..10c3d56 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -264,9 +264,6 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> return false;
> }
>
> - if (intel_encoder_check_is_cloned(&lvds_encoder->base))
> - return false;
> -
> if ((I915_READ(lvds_encoder->reg) & LVDS_A3_POWER_MASK) ==
> LVDS_A3_POWER_UP)
> lvds_bpp = 8*3;
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 7d11a5a..39debd8 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -914,9 +914,6 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> if (!tv_mode)
> return false;
>
> - if (intel_encoder_check_is_cloned(&intel_tv->base))
> - return false;
> -
> pipe_config->adjusted_mode.clock = tv_mode->clock;
> DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
> pipe_config->pipe_bpp = 8*3;
> --
> 1.7.11.7
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 9+ messages in thread