* [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
@ 2017-10-09 16:19 Ville Syrjala
2017-10-09 16:19 ` [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode Ville Syrjala
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Ville Syrjala @ 2017-10-09 16:19 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reuse the normal state readout code to get the fixed mode for LVDS/DVO
encoders. This removes some partially duplicated state readout code
from LVDS/DVO encoders. The duplicated code wasn't actually even
populating the negative h/vsync flags, leading to possible state checker
complaints. The normal readout code populates that stuff fully.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++++-------------------
drivers/gpu/drm/i915/intel_drv.h | 5 ++--
drivers/gpu/drm/i915/intel_dvo.c | 33 ++++++------------------
drivers/gpu/drm/i915/intel_lvds.c | 18 ++++---------
4 files changed, 39 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 15844bf92434..f8693374955c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10247,48 +10247,44 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
&pipe_config->fdi_m_n);
}
-/** Returns the currently programmed mode of the given pipe. */
-struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
- struct drm_crtc *crtc)
+/* Returns the currently programmed mode of the given encoder. */
+struct drm_display_mode *
+intel_encoder_current_mode(struct intel_encoder *encoder)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_crtc_state *crtc_state;
struct drm_display_mode *mode;
- struct intel_crtc_state *pipe_config;
- enum pipe pipe = intel_crtc->pipe;
+ struct intel_crtc *crtc;
+ enum pipe pipe;
+
+ if (!encoder->get_hw_state(encoder, &pipe))
+ return NULL;
+
+ crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
mode = kzalloc(sizeof(*mode), GFP_KERNEL);
if (!mode)
return NULL;
- pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
- if (!pipe_config) {
+ crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
+ if (!crtc_state) {
kfree(mode);
return NULL;
}
- /*
- * Construct a pipe_config sufficient for getting the clock info
- * back out of crtc_clock_get.
- *
- * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
- * to use a real value here instead.
- */
- pipe_config->cpu_transcoder = (enum transcoder) pipe;
- pipe_config->pixel_multiplier = 1;
- pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
- pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
- pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
- i9xx_crtc_clock_get(intel_crtc, pipe_config);
+ crtc_state->base.crtc = &crtc->base;
- pipe_config->base.adjusted_mode.crtc_clock =
- pipe_config->port_clock / pipe_config->pixel_multiplier;
+ if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
+ kfree(crtc_state);
+ kfree(mode);
+ return NULL;
+ }
- intel_get_pipe_timings(intel_crtc, pipe_config);
+ encoder->get_config(encoder, crtc_state);
- intel_mode_from_pipe_config(mode, pipe_config);
+ intel_mode_from_pipe_config(mode, crtc_state);
- kfree(pipe_config);
+ kfree(crtc_state);
return mode;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0cab667fff57..b57a691409c4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1363,8 +1363,9 @@ struct intel_connector *intel_connector_alloc(void);
bool intel_connector_get_hw_state(struct intel_connector *connector);
void intel_connector_attach_encoder(struct intel_connector *connector,
struct intel_encoder *encoder);
-struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
- struct drm_crtc *crtc);
+struct drm_display_mode *
+intel_encoder_current_mode(struct intel_encoder *encoder);
+
enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 5c562e1f56ae..53c9b763f4ce 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -379,32 +379,15 @@ static const struct drm_encoder_funcs intel_dvo_enc_funcs = {
* chip being on DVOB/C and having multiple pipes.
*/
static struct drm_display_mode *
-intel_dvo_get_current_mode(struct drm_connector *connector)
+intel_dvo_get_current_mode(struct intel_encoder *encoder)
{
- struct drm_device *dev = connector->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
- uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
- struct drm_display_mode *mode = NULL;
+ struct drm_display_mode *mode;
- /* If the DVO port is active, that'll be the LVDS, so we can pull out
- * its timings to get how the BIOS set up the panel.
- */
- if (dvo_val & DVO_ENABLE) {
- struct intel_crtc *crtc;
- int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
-
- crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
- if (crtc) {
- mode = intel_crtc_mode_get(dev, &crtc->base);
- if (mode) {
- mode->type |= DRM_MODE_TYPE_PREFERRED;
- if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
- mode->flags |= DRM_MODE_FLAG_PHSYNC;
- if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
- mode->flags |= DRM_MODE_FLAG_PVSYNC;
- }
- }
+ mode = intel_encoder_current_mode(encoder);
+ if (mode) {
+ DRM_DEBUG_KMS("using current (BIOS) mode: ");
+ drm_mode_debug_printmodeline(mode);
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
}
return mode;
@@ -551,7 +534,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
* mode being output through DVO.
*/
intel_panel_init(&intel_connector->panel,
- intel_dvo_get_current_mode(connector),
+ intel_dvo_get_current_mode(intel_encoder),
NULL, NULL);
intel_dvo->panel_wants_dither = true;
}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index a55954a89148..c5072b951b9c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -939,10 +939,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
struct drm_display_mode *fixed_mode = NULL;
struct drm_display_mode *downclock_mode = NULL;
struct edid *edid;
- struct intel_crtc *crtc;
i915_reg_t lvds_reg;
u32 lvds;
- int pipe;
u8 pin;
u32 allowed_scalers;
@@ -1118,17 +1116,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
if (HAS_PCH_SPLIT(dev_priv))
goto failed;
- pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
- crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
-
- if (crtc && (lvds & LVDS_PORT_EN)) {
- fixed_mode = intel_crtc_mode_get(dev, &crtc->base);
- if (fixed_mode) {
- DRM_DEBUG_KMS("using current (BIOS) mode: ");
- drm_mode_debug_printmodeline(fixed_mode);
- fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
- goto out;
- }
+ fixed_mode = intel_encoder_current_mode(intel_encoder);
+ if (fixed_mode) {
+ DRM_DEBUG_KMS("using current (BIOS) mode: ");
+ drm_mode_debug_printmodeline(fixed_mode);
+ fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
}
/* If we still don't have a mode after all that, give up. */
--
2.13.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode
2017-10-09 16:19 [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Ville Syrjala
@ 2017-10-09 16:19 ` Ville Syrjala
2017-10-11 18:06 ` Chris Wilson
2017-10-09 17:00 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Patchwork
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjala @ 2017-10-09 16:19 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
With intel_encoder_current_mode() using the normal state readout code it
actually works on PCH platforms as well. So let's nuke the PCH check from
intel_lvds_init(). I suppose there aren't any machines that actually
need this, but at least we get to eliminate a few lines of code, and one
FIXME.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_lvds.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index c5072b951b9c..38572d65e46e 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1111,11 +1111,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
* on. If so, assume that whatever is currently programmed is the
* correct mode.
*/
-
- /* Ironlake: FIXME if still fail, not try pipe mode now */
- if (HAS_PCH_SPLIT(dev_priv))
- goto failed;
-
fixed_mode = intel_encoder_current_mode(intel_encoder);
if (fixed_mode) {
DRM_DEBUG_KMS("using current (BIOS) mode: ");
--
2.13.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-09 16:19 [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Ville Syrjala
2017-10-09 16:19 ` [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode Ville Syrjala
@ 2017-10-09 17:00 ` Patchwork
2017-10-09 21:39 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-10 14:33 ` [PATCH 1/2] " Chris Wilson
3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-10-09 17:00 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
URL : https://patchwork.freedesktop.org/series/31599/
State : success
== Summary ==
Series 31599v1 series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
https://patchwork.freedesktop.org/api/1.0/series/31599/revisions/1/mbox/
Test kms_cursor_legacy:
Subgroup basic-flip-after-cursor-legacy:
incomplete -> PASS (fi-glk-1)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS (fi-skl-6700k)
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:450s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:465s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:389s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:560s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:284s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:519s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:528s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:532s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:516s
fi-cfl-s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 time:555s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:621s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:593s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:434s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:422s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:458s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:506s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:473s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:504s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:581s
fi-kbl-7567u total:289 pass:265 dwarn:4 dfail:0 fail:0 skip:20 time:487s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:663s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:467s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:655s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:530s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:507s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:467s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:573s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:430s
080df4bff8208a891f31629d6415ff29f6d7931a drm-tip: 2017y-10m-09d-15h-14m-40s UTC integration manifest
22fa1ff9ae0c drm/i915: Allow PCH platforms fall back to BIOS LVDS mode
a8af7891506e drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5957/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-09 16:19 [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Ville Syrjala
2017-10-09 16:19 ` [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode Ville Syrjala
2017-10-09 17:00 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Patchwork
@ 2017-10-09 21:39 ` Patchwork
2017-10-10 14:33 ` [PATCH 1/2] " Chris Wilson
3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-10-09 21:39 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
URL : https://patchwork.freedesktop.org/series/31599/
State : success
== Summary ==
Test perf:
Subgroup blocking:
pass -> FAIL (shard-hsw) fdo#102252
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-hsw total:2482 pass:1347 dwarn:23 dfail:0 fail:9 skip:1103 time:9822s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5957/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-09 16:19 [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Ville Syrjala
` (2 preceding siblings ...)
2017-10-09 21:39 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-10-10 14:33 ` Chris Wilson
2017-10-10 14:55 ` Ville Syrjälä
2017-10-11 16:21 ` Chris Wilson
3 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2017-10-10 14:33 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Quoting Ville Syrjala (2017-10-09 17:19:50)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reuse the normal state readout code to get the fixed mode for LVDS/DVO
> encoders. This removes some partially duplicated state readout code
> from LVDS/DVO encoders. The duplicated code wasn't actually even
> populating the negative h/vsync flags, leading to possible state checker
> complaints. The normal readout code populates that stuff fully.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++++-------------------
> drivers/gpu/drm/i915/intel_drv.h | 5 ++--
> drivers/gpu/drm/i915/intel_dvo.c | 33 ++++++------------------
> drivers/gpu/drm/i915/intel_lvds.c | 18 ++++---------
> 4 files changed, 39 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 15844bf92434..f8693374955c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10247,48 +10247,44 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> &pipe_config->fdi_m_n);
> }
>
> -/** Returns the currently programmed mode of the given pipe. */
> -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc)
> +/* Returns the currently programmed mode of the given encoder. */
> +struct drm_display_mode *
> +intel_encoder_current_mode(struct intel_encoder *encoder)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc_state *crtc_state;
> struct drm_display_mode *mode;
> - struct intel_crtc_state *pipe_config;
> - enum pipe pipe = intel_crtc->pipe;
> + struct intel_crtc *crtc;
> + enum pipe pipe;
> +
> + if (!encoder->get_hw_state(encoder, &pipe))
> + return NULL;
There's no chance that get_hw_state can return a pipe beyond our
knowledge? I'm presuming we are part of the early hw setup here, so may
not have sanitized everything?
> +
> + crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>
> mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> if (!mode)
> return NULL;
>
> - pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> - if (!pipe_config) {
> + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> + if (!crtc_state) {
> kfree(mode);
> return NULL;
> }
>
> - /*
> - * Construct a pipe_config sufficient for getting the clock info
> - * back out of crtc_clock_get.
> - *
> - * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
> - * to use a real value here instead.
> - */
> - pipe_config->cpu_transcoder = (enum transcoder) pipe;
> - pipe_config->pixel_multiplier = 1;
> - pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> - pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> - pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> - i9xx_crtc_clock_get(intel_crtc, pipe_config);
> + crtc_state->base.crtc = &crtc->base;
>
> - pipe_config->base.adjusted_mode.crtc_clock =
> - pipe_config->port_clock / pipe_config->pixel_multiplier;
> + if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
> + kfree(crtc_state);
> + kfree(mode);
> + return NULL;
> + }
>
> - intel_get_pipe_timings(intel_crtc, pipe_config);
> + encoder->get_config(encoder, crtc_state);
>
> - intel_mode_from_pipe_config(mode, pipe_config);
> + intel_mode_from_pipe_config(mode, crtc_state);
>
> - kfree(pipe_config);
> + kfree(crtc_state);
This all looks consistent to my eyes.
>
> return mode;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0cab667fff57..b57a691409c4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1363,8 +1363,9 @@ struct intel_connector *intel_connector_alloc(void);
> bool intel_connector_get_hw_state(struct intel_connector *connector);
> void intel_connector_attach_encoder(struct intel_connector *connector,
> struct intel_encoder *encoder);
> -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc);
> +struct drm_display_mode *
> +intel_encoder_current_mode(struct intel_encoder *encoder);
> +
> enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 5c562e1f56ae..53c9b763f4ce 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -379,32 +379,15 @@ static const struct drm_encoder_funcs intel_dvo_enc_funcs = {
> * chip being on DVOB/C and having multiple pipes.
> */
> static struct drm_display_mode *
> -intel_dvo_get_current_mode(struct drm_connector *connector)
> +intel_dvo_get_current_mode(struct intel_encoder *encoder)
> {
> - struct drm_device *dev = connector->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> - uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> - struct drm_display_mode *mode = NULL;
> + struct drm_display_mode *mode;
>
> - /* If the DVO port is active, that'll be the LVDS, so we can pull out
> - * its timings to get how the BIOS set up the panel.
> - */
> - if (dvo_val & DVO_ENABLE) {
> - struct intel_crtc *crtc;
> - int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
> -
> - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> - if (crtc) {
> - mode = intel_crtc_mode_get(dev, &crtc->base);
> - if (mode) {
> - mode->type |= DRM_MODE_TYPE_PREFERRED;
> - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> - }
> - }
> + mode = intel_encoder_current_mode(encoder);
Ok.
> + if (mode) {
> + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> + drm_mode_debug_printmodeline(mode);
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
> }
>
> return mode;
> @@ -551,7 +534,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> * mode being output through DVO.
> */
> intel_panel_init(&intel_connector->panel,
> - intel_dvo_get_current_mode(connector),
> + intel_dvo_get_current_mode(intel_encoder),
> NULL, NULL);
> intel_dvo->panel_wants_dither = true;
> }
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index a55954a89148..c5072b951b9c 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -939,10 +939,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> struct drm_display_mode *fixed_mode = NULL;
> struct drm_display_mode *downclock_mode = NULL;
> struct edid *edid;
> - struct intel_crtc *crtc;
> i915_reg_t lvds_reg;
> u32 lvds;
> - int pipe;
> u8 pin;
> u32 allowed_scalers;
>
> @@ -1118,17 +1116,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> if (HAS_PCH_SPLIT(dev_priv))
> goto failed;
>
> - pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
> - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -
> - if (crtc && (lvds & LVDS_PORT_EN)) {
> - fixed_mode = intel_crtc_mode_get(dev, &crtc->base);
> - if (fixed_mode) {
> - DRM_DEBUG_KMS("using current (BIOS) mode: ");
> - drm_mode_debug_printmodeline(fixed_mode);
> - fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> - goto out;
> - }
> + fixed_mode = intel_encoder_current_mode(intel_encoder);
> + if (fixed_mode) {
> + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> + drm_mode_debug_printmodeline(fixed_mode);
> + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> }
>
> /* If we still don't have a mode after all that, give up. */
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
I will give it a spin shortly (give or take compilation times on a slow
machine and forgetfulness).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-10 14:33 ` [PATCH 1/2] " Chris Wilson
@ 2017-10-10 14:55 ` Ville Syrjälä
2017-10-11 16:21 ` Chris Wilson
1 sibling, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2017-10-10 14:55 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Tue, Oct 10, 2017 at 03:33:33PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-10-09 17:19:50)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reuse the normal state readout code to get the fixed mode for LVDS/DVO
> > encoders. This removes some partially duplicated state readout code
> > from LVDS/DVO encoders. The duplicated code wasn't actually even
> > populating the negative h/vsync flags, leading to possible state checker
> > complaints. The normal readout code populates that stuff fully.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++++-------------------
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++--
> > drivers/gpu/drm/i915/intel_dvo.c | 33 ++++++------------------
> > drivers/gpu/drm/i915/intel_lvds.c | 18 ++++---------
> > 4 files changed, 39 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 15844bf92434..f8693374955c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10247,48 +10247,44 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> > &pipe_config->fdi_m_n);
> > }
> >
> > -/** Returns the currently programmed mode of the given pipe. */
> > -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > - struct drm_crtc *crtc)
> > +/* Returns the currently programmed mode of the given encoder. */
> > +struct drm_display_mode *
> > +intel_encoder_current_mode(struct intel_encoder *encoder)
> > {
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_crtc_state *crtc_state;
> > struct drm_display_mode *mode;
> > - struct intel_crtc_state *pipe_config;
> > - enum pipe pipe = intel_crtc->pipe;
> > + struct intel_crtc *crtc;
> > + enum pipe pipe;
> > +
> > + if (!encoder->get_hw_state(encoder, &pipe))
> > + return NULL;
>
> There's no chance that get_hw_state can return a pipe beyond our
> knowledge?
It shouldn't, unless the hardware registers are plain lying to us.
It just checks the port register whether the port is enabled, and
which pipe is connected to it.
> I'm presuming we are part of the early hw setup here, so may
> not have sanitized everything?
I think we should have done enough setup by this time. And we do the
normal state readout very shortly afterwards, so things had better be
ready by then.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-10 14:33 ` [PATCH 1/2] " Chris Wilson
2017-10-10 14:55 ` Ville Syrjälä
@ 2017-10-11 16:21 ` Chris Wilson
2017-10-11 17:18 ` Ville Syrjälä
1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-10-11 16:21 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Quoting Chris Wilson (2017-10-10 15:33:33)
> Quoting Ville Syrjala (2017-10-09 17:19:50)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reuse the normal state readout code to get the fixed mode for LVDS/DVO
> > encoders. This removes some partially duplicated state readout code
> > from LVDS/DVO encoders. The duplicated code wasn't actually even
> > populating the negative h/vsync flags, leading to possible state checker
> > complaints. The normal readout code populates that stuff fully.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++++-------------------
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++--
> > drivers/gpu/drm/i915/intel_dvo.c | 33 ++++++------------------
> > drivers/gpu/drm/i915/intel_lvds.c | 18 ++++---------
> > 4 files changed, 39 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 15844bf92434..f8693374955c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10247,48 +10247,44 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> > &pipe_config->fdi_m_n);
> > }
> >
> > -/** Returns the currently programmed mode of the given pipe. */
> > -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > - struct drm_crtc *crtc)
> > +/* Returns the currently programmed mode of the given encoder. */
> > +struct drm_display_mode *
> > +intel_encoder_current_mode(struct intel_encoder *encoder)
> > {
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_crtc_state *crtc_state;
> > struct drm_display_mode *mode;
> > - struct intel_crtc_state *pipe_config;
> > - enum pipe pipe = intel_crtc->pipe;
> > + struct intel_crtc *crtc;
> > + enum pipe pipe;
> > +
> > + if (!encoder->get_hw_state(encoder, &pipe))
> > + return NULL;
>
> There's no chance that get_hw_state can return a pipe beyond our
> knowledge? I'm presuming we are part of the early hw setup here, so may
> not have sanitized everything?
>
> > +
> > + crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> >
> > mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> > if (!mode)
> > return NULL;
> >
> > - pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> > - if (!pipe_config) {
> > + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> > + if (!crtc_state) {
> > kfree(mode);
> > return NULL;
> > }
> >
> > - /*
> > - * Construct a pipe_config sufficient for getting the clock info
> > - * back out of crtc_clock_get.
> > - *
> > - * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
> > - * to use a real value here instead.
> > - */
> > - pipe_config->cpu_transcoder = (enum transcoder) pipe;
> > - pipe_config->pixel_multiplier = 1;
> > - pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> > - pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> > - pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> > - i9xx_crtc_clock_get(intel_crtc, pipe_config);
> > + crtc_state->base.crtc = &crtc->base;
> >
> > - pipe_config->base.adjusted_mode.crtc_clock =
> > - pipe_config->port_clock / pipe_config->pixel_multiplier;
> > + if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
> > + kfree(crtc_state);
> > + kfree(mode);
> > + return NULL;
> > + }
> >
> > - intel_get_pipe_timings(intel_crtc, pipe_config);
> > + encoder->get_config(encoder, crtc_state);
> >
> > - intel_mode_from_pipe_config(mode, pipe_config);
> > + intel_mode_from_pipe_config(mode, crtc_state);
> >
> > - kfree(pipe_config);
> > + kfree(crtc_state);
>
> This all looks consistent to my eyes.
> >
> > return mode;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 0cab667fff57..b57a691409c4 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1363,8 +1363,9 @@ struct intel_connector *intel_connector_alloc(void);
> > bool intel_connector_get_hw_state(struct intel_connector *connector);
> > void intel_connector_attach_encoder(struct intel_connector *connector,
> > struct intel_encoder *encoder);
> > -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > - struct drm_crtc *crtc);
> > +struct drm_display_mode *
> > +intel_encoder_current_mode(struct intel_encoder *encoder);
> > +
> > enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
> > int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> > struct drm_file *file_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index 5c562e1f56ae..53c9b763f4ce 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -379,32 +379,15 @@ static const struct drm_encoder_funcs intel_dvo_enc_funcs = {
> > * chip being on DVOB/C and having multiple pipes.
> > */
> > static struct drm_display_mode *
> > -intel_dvo_get_current_mode(struct drm_connector *connector)
> > +intel_dvo_get_current_mode(struct intel_encoder *encoder)
> > {
> > - struct drm_device *dev = connector->dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > - struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> > - uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> > - struct drm_display_mode *mode = NULL;
> > + struct drm_display_mode *mode;
> >
> > - /* If the DVO port is active, that'll be the LVDS, so we can pull out
> > - * its timings to get how the BIOS set up the panel.
> > - */
> > - if (dvo_val & DVO_ENABLE) {
> > - struct intel_crtc *crtc;
> > - int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
> > -
> > - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > - if (crtc) {
> > - mode = intel_crtc_mode_get(dev, &crtc->base);
> > - if (mode) {
> > - mode->type |= DRM_MODE_TYPE_PREFERRED;
> > - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> > - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> > - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> > - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> > - }
> > - }
> > + mode = intel_encoder_current_mode(encoder);
>
> Ok.
>
> > + if (mode) {
> > + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > + drm_mode_debug_printmodeline(mode);
> > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> > }
> >
> > return mode;
> > @@ -551,7 +534,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> > * mode being output through DVO.
> > */
> > intel_panel_init(&intel_connector->panel,
> > - intel_dvo_get_current_mode(connector),
> > + intel_dvo_get_current_mode(intel_encoder),
> > NULL, NULL);
> > intel_dvo->panel_wants_dither = true;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index a55954a89148..c5072b951b9c 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -939,10 +939,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > struct drm_display_mode *fixed_mode = NULL;
> > struct drm_display_mode *downclock_mode = NULL;
> > struct edid *edid;
> > - struct intel_crtc *crtc;
> > i915_reg_t lvds_reg;
> > u32 lvds;
> > - int pipe;
> > u8 pin;
> > u32 allowed_scalers;
> >
> > @@ -1118,17 +1116,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > if (HAS_PCH_SPLIT(dev_priv))
> > goto failed;
> >
> > - pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
> > - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > -
> > - if (crtc && (lvds & LVDS_PORT_EN)) {
> > - fixed_mode = intel_crtc_mode_get(dev, &crtc->base);
> > - if (fixed_mode) {
> > - DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > - drm_mode_debug_printmodeline(fixed_mode);
> > - fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > - goto out;
> > - }
> > + fixed_mode = intel_encoder_current_mode(intel_encoder);
> > + if (fixed_mode) {
> > + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > + drm_mode_debug_printmodeline(fixed_mode);
> > + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > }
> >
> > /* If we still don't have a mode after all that, give up. */
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> I will give it a spin shortly (give or take compilation times on a slow
> machine and forgetfulness).
Compiled, booted, and lightly toasted,
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode
2017-10-11 16:21 ` Chris Wilson
@ 2017-10-11 17:18 ` Ville Syrjälä
0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2017-10-11 17:18 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Oct 11, 2017 at 05:21:56PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2017-10-10 15:33:33)
> > Quoting Ville Syrjala (2017-10-09 17:19:50)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Reuse the normal state readout code to get the fixed mode for LVDS/DVO
> > > encoders. This removes some partially duplicated state readout code
> > > from LVDS/DVO encoders. The duplicated code wasn't actually even
> > > populating the negative h/vsync flags, leading to possible state checker
> > > complaints. The normal readout code populates that stuff fully.
> > >
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 50 +++++++++++++++++-------------------
> > > drivers/gpu/drm/i915/intel_drv.h | 5 ++--
> > > drivers/gpu/drm/i915/intel_dvo.c | 33 ++++++------------------
> > > drivers/gpu/drm/i915/intel_lvds.c | 18 ++++---------
> > > 4 files changed, 39 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 15844bf92434..f8693374955c 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -10247,48 +10247,44 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> > > &pipe_config->fdi_m_n);
> > > }
> > >
> > > -/** Returns the currently programmed mode of the given pipe. */
> > > -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > > - struct drm_crtc *crtc)
> > > +/* Returns the currently programmed mode of the given encoder. */
> > > +struct drm_display_mode *
> > > +intel_encoder_current_mode(struct intel_encoder *encoder)
> > > {
> > > - struct drm_i915_private *dev_priv = to_i915(dev);
> > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > + struct intel_crtc_state *crtc_state;
> > > struct drm_display_mode *mode;
> > > - struct intel_crtc_state *pipe_config;
> > > - enum pipe pipe = intel_crtc->pipe;
> > > + struct intel_crtc *crtc;
> > > + enum pipe pipe;
> > > +
> > > + if (!encoder->get_hw_state(encoder, &pipe))
> > > + return NULL;
> >
> > There's no chance that get_hw_state can return a pipe beyond our
> > knowledge? I'm presuming we are part of the early hw setup here, so may
> > not have sanitized everything?
> >
> > > +
> > > + crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > >
> > > mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> > > if (!mode)
> > > return NULL;
> > >
> > > - pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> > > - if (!pipe_config) {
> > > + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> > > + if (!crtc_state) {
> > > kfree(mode);
> > > return NULL;
> > > }
> > >
> > > - /*
> > > - * Construct a pipe_config sufficient for getting the clock info
> > > - * back out of crtc_clock_get.
> > > - *
> > > - * Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
> > > - * to use a real value here instead.
> > > - */
> > > - pipe_config->cpu_transcoder = (enum transcoder) pipe;
> > > - pipe_config->pixel_multiplier = 1;
> > > - pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> > > - pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> > > - pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> > > - i9xx_crtc_clock_get(intel_crtc, pipe_config);
> > > + crtc_state->base.crtc = &crtc->base;
> > >
> > > - pipe_config->base.adjusted_mode.crtc_clock =
> > > - pipe_config->port_clock / pipe_config->pixel_multiplier;
> > > + if (!dev_priv->display.get_pipe_config(crtc, crtc_state)) {
> > > + kfree(crtc_state);
> > > + kfree(mode);
> > > + return NULL;
> > > + }
> > >
> > > - intel_get_pipe_timings(intel_crtc, pipe_config);
> > > + encoder->get_config(encoder, crtc_state);
> > >
> > > - intel_mode_from_pipe_config(mode, pipe_config);
> > > + intel_mode_from_pipe_config(mode, crtc_state);
> > >
> > > - kfree(pipe_config);
> > > + kfree(crtc_state);
> >
> > This all looks consistent to my eyes.
> > >
> > > return mode;
> > > }
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 0cab667fff57..b57a691409c4 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1363,8 +1363,9 @@ struct intel_connector *intel_connector_alloc(void);
> > > bool intel_connector_get_hw_state(struct intel_connector *connector);
> > > void intel_connector_attach_encoder(struct intel_connector *connector,
> > > struct intel_encoder *encoder);
> > > -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> > > - struct drm_crtc *crtc);
> > > +struct drm_display_mode *
> > > +intel_encoder_current_mode(struct intel_encoder *encoder);
> > > +
> > > enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
> > > int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> > > struct drm_file *file_priv);
> > > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > > index 5c562e1f56ae..53c9b763f4ce 100644
> > > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > > @@ -379,32 +379,15 @@ static const struct drm_encoder_funcs intel_dvo_enc_funcs = {
> > > * chip being on DVOB/C and having multiple pipes.
> > > */
> > > static struct drm_display_mode *
> > > -intel_dvo_get_current_mode(struct drm_connector *connector)
> > > +intel_dvo_get_current_mode(struct intel_encoder *encoder)
> > > {
> > > - struct drm_device *dev = connector->dev;
> > > - struct drm_i915_private *dev_priv = to_i915(dev);
> > > - struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> > > - uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> > > - struct drm_display_mode *mode = NULL;
> > > + struct drm_display_mode *mode;
> > >
> > > - /* If the DVO port is active, that'll be the LVDS, so we can pull out
> > > - * its timings to get how the BIOS set up the panel.
> > > - */
> > > - if (dvo_val & DVO_ENABLE) {
> > > - struct intel_crtc *crtc;
> > > - int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
> > > -
> > > - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > > - if (crtc) {
> > > - mode = intel_crtc_mode_get(dev, &crtc->base);
> > > - if (mode) {
> > > - mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> > > - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> > > - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> > > - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> > > - }
> > > - }
> > > + mode = intel_encoder_current_mode(encoder);
> >
> > Ok.
> >
> > > + if (mode) {
> > > + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > > + drm_mode_debug_printmodeline(mode);
> > > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > }
> > >
> > > return mode;
> > > @@ -551,7 +534,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> > > * mode being output through DVO.
> > > */
> > > intel_panel_init(&intel_connector->panel,
> > > - intel_dvo_get_current_mode(connector),
> > > + intel_dvo_get_current_mode(intel_encoder),
> > > NULL, NULL);
> > > intel_dvo->panel_wants_dither = true;
> > > }
> > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > > index a55954a89148..c5072b951b9c 100644
> > > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > > @@ -939,10 +939,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > > struct drm_display_mode *fixed_mode = NULL;
> > > struct drm_display_mode *downclock_mode = NULL;
> > > struct edid *edid;
> > > - struct intel_crtc *crtc;
> > > i915_reg_t lvds_reg;
> > > u32 lvds;
> > > - int pipe;
> > > u8 pin;
> > > u32 allowed_scalers;
> > >
> > > @@ -1118,17 +1116,11 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > > if (HAS_PCH_SPLIT(dev_priv))
> > > goto failed;
> > >
> > > - pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
> > > - crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > > -
> > > - if (crtc && (lvds & LVDS_PORT_EN)) {
> > > - fixed_mode = intel_crtc_mode_get(dev, &crtc->base);
> > > - if (fixed_mode) {
> > > - DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > > - drm_mode_debug_printmodeline(fixed_mode);
> > > - fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > - goto out;
> > > - }
> > > + fixed_mode = intel_encoder_current_mode(intel_encoder);
> > > + if (fixed_mode) {
> > > + DRM_DEBUG_KMS("using current (BIOS) mode: ");
> > > + drm_mode_debug_printmodeline(fixed_mode);
> > > + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > }
> > >
> > > /* If we still don't have a mode after all that, give up. */
> >
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > I will give it a spin shortly (give or take compilation times on a slow
> > machine and forgetfulness).
>
> Compiled, booted, and lightly toasted,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cool. Thanks for checking again. Patch pushed to dinq.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode
2017-10-09 16:19 ` [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode Ville Syrjala
@ 2017-10-11 18:06 ` Chris Wilson
2017-10-11 18:47 ` Ville Syrjälä
0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-10-11 18:06 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Quoting Ville Syrjala (2017-10-09 17:19:51)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> With intel_encoder_current_mode() using the normal state readout code it
> actually works on PCH platforms as well. So let's nuke the PCH check from
> intel_lvds_init(). I suppose there aren't any machines that actually
> need this, but at least we get to eliminate a few lines of code, and one
> FIXME.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I have to admit the FIXME comment confused me utterly. Having read the
function, this should now work given the conversion of
intel_encoder_current_mode() to use the common encoder->get_hw_state()
routine.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode
2017-10-11 18:06 ` Chris Wilson
@ 2017-10-11 18:47 ` Ville Syrjälä
0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2017-10-11 18:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Oct 11, 2017 at 07:06:45PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-10-09 17:19:51)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > With intel_encoder_current_mode() using the normal state readout code it
> > actually works on PCH platforms as well. So let's nuke the PCH check from
> > intel_lvds_init(). I suppose there aren't any machines that actually
> > need this, but at least we get to eliminate a few lines of code, and one
> > FIXME.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I have to admit the FIXME comment confused me utterly.
It does seem to be something not quite English.
> Having read the
> function, this should now work given the conversion of
> intel_encoder_current_mode() to use the common encoder->get_hw_state()
> routine.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Thanks. Pushed this one as well.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-11 18:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-09 16:19 [PATCH 1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Ville Syrjala
2017-10-09 16:19 ` [PATCH 2/2] drm/i915: Allow PCH platforms fall back to BIOS LVDS mode Ville Syrjala
2017-10-11 18:06 ` Chris Wilson
2017-10-11 18:47 ` Ville Syrjälä
2017-10-09 17:00 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Reuse normal state readout for LVDS/DVO fixed mode Patchwork
2017-10-09 21:39 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-10 14:33 ` [PATCH 1/2] " Chris Wilson
2017-10-10 14:55 ` Ville Syrjälä
2017-10-11 16:21 ` Chris Wilson
2017-10-11 17:18 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.