* [PATCH 1/4] drm/i915: Pre-compute pipe enabled state
2014-01-10 9:28 [PATCH 0/4] drm/i915: Small step towards atomic modeset ville.syrjala
@ 2014-01-10 9:28 ` ville.syrjala
2014-01-10 9:28 ` [PATCH 2/4] drm/i915: Prepare to track new pipe config per pipe ville.syrjala
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: ville.syrjala @ 2014-01-10 9:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add 'new_enabled' to intel_crtc and precompute it alongside new_encoder
and new_crtc. This will allow making decisions about shared resources
that are affected by the set of active pipes, before we've clobbered
anything for real.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/intel_drv.h | 2 +
2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bf4c6b5..7020b7b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8736,6 +8736,7 @@ static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
*/
static void intel_modeset_update_staged_output_state(struct drm_device *dev)
{
+ struct intel_crtc *crtc;
struct intel_encoder *encoder;
struct intel_connector *connector;
@@ -8750,6 +8751,11 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
encoder->new_crtc =
to_intel_crtc(encoder->base.crtc);
}
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+ base.head) {
+ crtc->new_enabled = crtc->base.enabled;
+ }
}
/**
@@ -8759,6 +8765,7 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
*/
static void intel_modeset_commit_output_state(struct drm_device *dev)
{
+ struct intel_crtc *crtc;
struct intel_encoder *encoder;
struct intel_connector *connector;
@@ -8771,6 +8778,11 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
base.head) {
encoder->base.crtc = &encoder->new_crtc->base;
}
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+ base.head) {
+ crtc->base.enabled = crtc->new_enabled;
+ }
}
static void
@@ -9097,29 +9109,22 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
*prepare_pipes |= 1 << encoder->new_crtc->pipe;
}
- /* Check for any pipes that will be fully disabled ... */
+ /* Check for pipes that will be enabled/disabled ... */
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
- bool used = false;
-
- /* Don't try to disable disabled crtcs. */
- if (!intel_crtc->base.enabled)
+ if (intel_crtc->base.enabled == intel_crtc->new_enabled)
continue;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list,
- base.head) {
- if (encoder->new_crtc == intel_crtc)
- used = true;
- }
-
- if (!used)
+ if (!intel_crtc->new_enabled)
*disable_pipes |= 1 << intel_crtc->pipe;
+ else
+ *prepare_pipes |= 1 << intel_crtc->pipe;
}
/* set_mode is also used to update properties on life display pipes. */
intel_crtc = to_intel_crtc(crtc);
- if (crtc->enabled)
+ if (intel_crtc->new_enabled)
*prepare_pipes |= 1 << intel_crtc->pipe;
/*
@@ -9178,10 +9183,10 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
intel_modeset_commit_output_state(dev);
- /* Update computed state. */
+ /* Double check state. */
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
- intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base);
+ WARN_ON(intel_crtc->base.enabled != intel_crtc_in_use(&intel_crtc->base));
}
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -9723,16 +9728,24 @@ static void intel_set_config_free(struct intel_set_config *config)
kfree(config->save_connector_encoders);
kfree(config->save_encoder_crtcs);
+ kfree(config->save_crtc_enabled);
kfree(config);
}
static int intel_set_config_save_state(struct drm_device *dev,
struct intel_set_config *config)
{
+ struct drm_crtc *crtc;
struct drm_encoder *encoder;
struct drm_connector *connector;
int count;
+ config->save_crtc_enabled =
+ kcalloc(dev->mode_config.num_crtc,
+ sizeof(bool), GFP_KERNEL);
+ if (!config->save_crtc_enabled)
+ return -ENOMEM;
+
config->save_encoder_crtcs =
kcalloc(dev->mode_config.num_encoder,
sizeof(struct drm_crtc *), GFP_KERNEL);
@@ -9750,6 +9763,11 @@ static int intel_set_config_save_state(struct drm_device *dev,
* restored, not the drivers personal bookkeeping.
*/
count = 0;
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ config->save_crtc_enabled[count++] = crtc->enabled;
+ }
+
+ count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
config->save_encoder_crtcs[count++] = encoder->crtc;
}
@@ -9765,11 +9783,17 @@ static int intel_set_config_save_state(struct drm_device *dev,
static void intel_set_config_restore_state(struct drm_device *dev,
struct intel_set_config *config)
{
+ struct intel_crtc *crtc;
struct intel_encoder *encoder;
struct intel_connector *connector;
int count;
count = 0;
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+ crtc->new_enabled = config->save_crtc_enabled[count++];
+ }
+
+ count = 0;
list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
encoder->new_crtc =
to_intel_crtc(config->save_encoder_crtcs[count++]);
@@ -9853,9 +9877,9 @@ intel_modeset_stage_output_state(struct drm_device *dev,
struct drm_mode_set *set,
struct intel_set_config *config)
{
- struct drm_crtc *new_crtc;
struct intel_connector *connector;
struct intel_encoder *encoder;
+ struct intel_crtc *crtc;
int ro;
/* The upper layers ensure that we either disable a crtc or have a list
@@ -9898,6 +9922,8 @@ intel_modeset_stage_output_state(struct drm_device *dev,
/* Update crtc of enabled connectors. */
list_for_each_entry(connector, &dev->mode_config.connector_list,
base.head) {
+ struct drm_crtc *new_crtc;
+
if (!connector->new_encoder)
continue;
@@ -9944,6 +9970,26 @@ next_encoder:
}
/* Now we've also updated encoder->new_crtc for all encoders. */
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list,
+ base.head) {
+ crtc->new_enabled = false;
+
+ list_for_each_entry(encoder,
+ &dev->mode_config.encoder_list,
+ base.head) {
+ if (encoder->new_crtc == crtc) {
+ crtc->new_enabled = true;
+ break;
+ }
+ }
+
+ if (crtc->new_enabled != crtc->base.enabled) {
+ DRM_DEBUG_KMS("crtc %sabled, full mode switch\n",
+ crtc->new_enabled ? "en" : "dis");
+ config->mode_changed = true;
+ }
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 46aea6c..7f55290 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -359,6 +359,7 @@ struct intel_crtc {
bool cursor_visible;
struct intel_crtc_config config;
+ bool new_enabled;
uint32_t ddi_pll_sel;
@@ -540,6 +541,7 @@ struct intel_unpin_work {
struct intel_set_config {
struct drm_encoder **save_connector_encoders;
struct drm_crtc **save_encoder_crtcs;
+ bool *save_crtc_enabled;
bool fb_changed;
bool mode_changed;
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] drm/i915: Prepare to track new pipe config per pipe
2014-01-10 9:28 [PATCH 0/4] drm/i915: Small step towards atomic modeset ville.syrjala
2014-01-10 9:28 ` [PATCH 1/4] drm/i915: Pre-compute pipe enabled state ville.syrjala
@ 2014-01-10 9:28 ` ville.syrjala
2014-01-10 9:28 ` [PATCH 3/4] drm/i915: Use new_config and new_enabled to simplify the VLV cdclk code ville.syrjala
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: ville.syrjala @ 2014-01-10 9:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add a new_config pointer to intel_crtc which will point to the new pipe
config for said crtc while intel_crtc.config will still contain the old
config during first parts of the modeset operation. This is a step
towards having the entire new state available during the compute phase,
so that we can make accurate decisions about global resource usage.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7020b7b..5861975 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9187,6 +9187,7 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
WARN_ON(intel_crtc->base.enabled != intel_crtc_in_use(&intel_crtc->base));
+ WARN_ON(intel_crtc->new_config != &intel_crtc->config);
}
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -9620,6 +9621,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
}
intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
"[modeset]");
+ to_intel_crtc(crtc)->new_config = pipe_config;
}
/*
@@ -9653,6 +9655,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
/* mode_set/enable/disable functions rely on a correct pipe
* config. */
to_intel_crtc(crtc)->config = *pipe_config;
+ to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;
}
/* Only after disabling all output pipelines that will be changed can we
@@ -10226,6 +10229,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
+ intel_crtc->new_config = &intel_crtc->config;
+
drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7f55290..d538657 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -359,6 +359,7 @@ struct intel_crtc {
bool cursor_visible;
struct intel_crtc_config config;
+ struct intel_crtc_config *new_config;
bool new_enabled;
uint32_t ddi_pll_sel;
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] drm/i915: Use new_config and new_enabled to simplify the VLV cdclk code
2014-01-10 9:28 [PATCH 0/4] drm/i915: Small step towards atomic modeset ville.syrjala
2014-01-10 9:28 ` [PATCH 1/4] drm/i915: Pre-compute pipe enabled state ville.syrjala
2014-01-10 9:28 ` [PATCH 2/4] drm/i915: Prepare to track new pipe config per pipe ville.syrjala
@ 2014-01-10 9:28 ` ville.syrjala
2014-01-10 9:28 ` [PATCH 4/4] drm/i915: Don't oops if the initial modeset fails ville.syrjala
2014-01-13 16:56 ` [PATCH 0/4] drm/i915: Small step towards atomic modeset Imre Deak
4 siblings, 0 replies; 6+ messages in thread
From: ville.syrjala @ 2014-01-10 9:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On VLV we need to compute the new cdclk before we've updated the current
state. The code achieved that in a somewhat complex way. Now that we
have new_enabled and new_config, we can simplify the code quite a bit.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5861975..9998bcb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4057,9 +4057,8 @@ static int valleyview_calc_cdclk(struct drm_i915_private *dev_priv,
/* Looks like the 200MHz CDclk freq doesn't work on some configs */
}
-static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
- unsigned modeset_pipes,
- struct intel_crtc_config *pipe_config)
+/* compute the max pixel clock for new configuration */
+static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
struct intel_crtc *intel_crtc;
@@ -4067,31 +4066,26 @@ static int intel_mode_max_pixclk(struct drm_i915_private *dev_priv,
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
- if (modeset_pipes & (1 << intel_crtc->pipe))
- max_pixclk = max(max_pixclk,
- pipe_config->adjusted_mode.crtc_clock);
- else if (intel_crtc->base.enabled)
+ if (intel_crtc->new_enabled)
max_pixclk = max(max_pixclk,
- intel_crtc->config.adjusted_mode.crtc_clock);
+ intel_crtc->new_config->adjusted_mode.crtc_clock);
}
return max_pixclk;
}
static void valleyview_modeset_global_pipes(struct drm_device *dev,
- unsigned *prepare_pipes,
- unsigned modeset_pipes,
- struct intel_crtc_config *pipe_config)
+ unsigned *prepare_pipes)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc;
- int max_pixclk = intel_mode_max_pixclk(dev_priv, modeset_pipes,
- pipe_config);
+ int max_pixclk = intel_mode_max_pixclk(dev_priv);
int cur_cdclk = valleyview_cur_cdclk(dev_priv);
if (valleyview_calc_cdclk(dev_priv, max_pixclk) == cur_cdclk)
return;
+ /* disable/enable all currently active pipes while we change cdclk */
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head)
if (intel_crtc->base.enabled)
@@ -4101,7 +4095,7 @@ static void valleyview_modeset_global_pipes(struct drm_device *dev,
static void valleyview_modeset_global_resources(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- int max_pixclk = intel_mode_max_pixclk(dev_priv, 0, NULL);
+ int max_pixclk = intel_mode_max_pixclk(dev_priv);
int cur_cdclk = valleyview_cur_cdclk(dev_priv);
int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
@@ -9632,8 +9626,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* adjusted_mode bits in the crtc directly.
*/
if (IS_VALLEYVIEW(dev)) {
- valleyview_modeset_global_pipes(dev, &prepare_pipes,
- modeset_pipes, pipe_config);
+ valleyview_modeset_global_pipes(dev, &prepare_pipes);
/* may have added more to prepare_pipes than we should */
prepare_pipes &= ~disable_pipes;
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] drm/i915: Don't oops if the initial modeset fails
2014-01-10 9:28 [PATCH 0/4] drm/i915: Small step towards atomic modeset ville.syrjala
` (2 preceding siblings ...)
2014-01-10 9:28 ` [PATCH 3/4] drm/i915: Use new_config and new_enabled to simplify the VLV cdclk code ville.syrjala
@ 2014-01-10 9:28 ` ville.syrjala
2014-01-13 16:56 ` [PATCH 0/4] drm/i915: Small step towards atomic modeset Imre Deak
4 siblings, 0 replies; 6+ messages in thread
From: ville.syrjala @ 2014-01-10 9:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
If the first modeset operation fails, we will attempt to restore the
previous configuration that we read out from the hardware. But as we
don't yet reconstruct the framebuffer information, we end up calling
the modeset code with an enabled crtc but with fb==NULL. This will
lead to an oops within the modeset code.
Check for NULL fb when restoring the configuration, and instead of
oopsing simply disable the pipe.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9998bcb..baac4be 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9989,6 +9989,29 @@ next_encoder:
return 0;
}
+static void disable_crtc_nofb(struct intel_crtc *crtc)
+{
+ struct drm_device *dev = crtc->base.dev;
+ struct intel_encoder *encoder;
+ struct intel_connector *connector;
+
+ DRM_DEBUG_KMS("Trying to restore without FB -> disabling pipe %c\n",
+ pipe_name(crtc->pipe));
+
+ list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) {
+ if (connector->new_encoder &&
+ connector->new_encoder->new_crtc == crtc)
+ connector->new_encoder = NULL;
+ }
+
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
+ if (encoder->new_crtc == crtc)
+ encoder->new_crtc = NULL;
+ }
+
+ crtc->new_enabled = false;
+}
+
static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
@@ -10065,6 +10088,15 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
fail:
intel_set_config_restore_state(dev, config);
+ /*
+ * HACK: if the pipe was on, but we didn't have a framebuffer,
+ * force the pipe off to avoid oopsing in the modeset code
+ * due to fb==NULL. This should only happen during boot since
+ * we don't yet reconstruct the FB from the hardware state.
+ */
+ if (to_intel_crtc(save_set.crtc)->new_enabled && !save_set.fb)
+ disable_crtc_nofb(to_intel_crtc(save_set.crtc));
+
/* Try to restore the config */
if (config->mode_changed &&
intel_set_mode(save_set.crtc, save_set.mode,
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] drm/i915: Small step towards atomic modeset
2014-01-10 9:28 [PATCH 0/4] drm/i915: Small step towards atomic modeset ville.syrjala
` (3 preceding siblings ...)
2014-01-10 9:28 ` [PATCH 4/4] drm/i915: Don't oops if the initial modeset fails ville.syrjala
@ 2014-01-13 16:56 ` Imre Deak
4 siblings, 0 replies; 6+ messages in thread
From: Imre Deak @ 2014-01-13 16:56 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 1531 bytes --]
On Fri, 2014-01-10 at 11:28 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Looks ok to me. On the series:
Reviewed-by: Imre Deak <imre.deak@intel.com>
>
> I started to think a bit about how the code should look for doing a full
> modeset for more than one pipe at a time. This is how far I got yesterday.
>
> I suppose the next thing would be moving the PLL calculations into compute
> config stage, and then actually computing a new pipe config for more than
> one pipe.
>
> After that we could improve the IVB bifurcate stuff to handle cases where
> pipe B is already enabled w/ > 2 FDI lanes, and then we try to light up
> pipe C. Assuming pipe B bandwidth can be reduced sufficiently to fit into
> two lanes, such an operation should succeed.
>
> But I think I'll try to finish off some of my other pending stuff before
> I continue with this.
>
> I fired this up on an IVB briefly and things seem fine. I didn't actually
> test the VLV cdclk change due to lack of hardware.
>
> Ville Syrjälä (4):
> drm/i915: Pre-compute pipe enabled state
> drm/i915: Prepare to track new pipe config per pipe
> drm/i915: Use new_config and new_enabled to simplify the VLV cdclk
> code
> drm/i915: Don't oops if the initial modeset fails
>
> drivers/gpu/drm/i915/intel_display.c | 140 +++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_drv.h | 3 +
> 2 files changed, 111 insertions(+), 32 deletions(-)
>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread