* [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.
@ 2017-08-09 19:48 Jim Bride
2017-08-09 20:14 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-10 13:13 ` [PATCH v6] " Jani Nikula
0 siblings, 2 replies; 6+ messages in thread
From: Jim Bride @ 2017-08-09 19:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Jani Nikula, Rodrigo Vivi
Some fixed resolution panels actually support more than one mode,
with the only thing different being the refresh rate. Having this
alternate mode available to us is desirable, because it allows us to
test PSR on panels whose setup time at the preferred mode is too long.
With this patch we allow the use of the alternate mode if it's
available and it was specifically requested.
v2 and v3: Rebase
v4: * Fix up some leaky mode stuff (Chris)
* Rebase
v5: * Fix a NULL pointer derefrence (David Weinehall)
v6: * Whitespace / spelling / checkpatch clean-up; no functional
change. (David)
* Rebase
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++-----
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_dsi.c | 2 +-
drivers/gpu/drm/i915/intel_dvo.c | 2 +-
drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
drivers/gpu/drm/i915/intel_panel.c | 6 ++++++
6 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 76c8a0b..576b5af 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1606,6 +1606,23 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
return bpp;
}
+static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
+ struct drm_display_mode *m2)
+{
+ bool bres = false;
+
+ if (m1 && m2)
+ bres = (m1->hdisplay == m2->hdisplay &&
+ m1->hsync_start == m2->hsync_start &&
+ m1->hsync_end == m2->hsync_end &&
+ m1->htotal == m2->htotal &&
+ m1->vdisplay == m2->vdisplay &&
+ m1->vsync_start == m2->vsync_start &&
+ m1->vsync_end == m2->vsync_end &&
+ m1->vtotal == m2->vtotal);
+ return bres;
+}
+
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
@@ -1652,8 +1669,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
- intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
- adjusted_mode);
+ struct drm_display_mode *panel_mode =
+ intel_connector->panel.alt_fixed_mode;
+ struct drm_display_mode *req_mode = &pipe_config->base.mode;
+
+ if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
+ panel_mode = intel_connector->panel.fixed_mode;
+
+ drm_mode_debug_printmodeline(panel_mode);
+
+ intel_fixed_panel_mode(panel_mode, adjusted_mode);
if (INTEL_GEN(dev_priv) >= 9) {
int ret;
@@ -5780,6 +5805,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_display_mode *fixed_mode = NULL;
+ struct drm_display_mode *alt_fixed_mode = NULL;
struct drm_display_mode *downclock_mode = NULL;
bool has_dpcd;
struct drm_display_mode *scan;
@@ -5835,13 +5861,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
}
intel_connector->edid = edid;
- /* prefer fixed mode from EDID if available */
+ /* prefer fixed mode from EDID if available, save an alt mode also */
list_for_each_entry(scan, &connector->probed_modes, head) {
if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
fixed_mode = drm_mode_duplicate(dev, scan);
downclock_mode = intel_dp_drrs_init(
intel_connector, fixed_mode);
- break;
+ } else if (!alt_fixed_mode) {
+ alt_fixed_mode = drm_mode_duplicate(dev, scan);
}
}
@@ -5878,7 +5905,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
pipe_name(pipe));
}
- intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+ intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
+ downclock_mode);
intel_connector->panel.backlight.power = intel_edp_backlight_power;
intel_panel_setup_backlight(connector, pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f91de9c..b3fcbb9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -265,6 +265,7 @@ struct intel_encoder {
struct intel_panel {
struct drm_display_mode *fixed_mode;
+ struct drm_display_mode *alt_fixed_mode;
struct drm_display_mode *downclock_mode;
/* backlight */
@@ -1679,6 +1680,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
/* intel_panel.c */
int intel_panel_init(struct intel_panel *panel,
struct drm_display_mode *fixed_mode,
+ struct drm_display_mode *alt_fixed_mode,
struct drm_display_mode *downclock_mode);
void intel_panel_fini(struct intel_panel *panel);
void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index b0b3adf..f0c11ae 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1849,7 +1849,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
connector->display_info.width_mm = fixed_mode->width_mm;
connector->display_info.height_mm = fixed_mode->height_mm;
- intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+ intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
intel_panel_setup_backlight(connector, INVALID_PIPE);
intel_dsi_add_properties(intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index baf369d..c0a0272 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -552,7 +552,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
*/
intel_panel_init(&intel_connector->panel,
intel_dvo_get_current_mode(connector),
- NULL);
+ 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 61d5579..8e21577 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1138,7 +1138,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
out:
mutex_unlock(&dev->mode_config.mutex);
- intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+ intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
+ downclock_mode);
intel_panel_setup_backlight(connector, INVALID_PIPE);
lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 88018fc..a17b1de 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1920,11 +1920,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
int intel_panel_init(struct intel_panel *panel,
struct drm_display_mode *fixed_mode,
+ struct drm_display_mode *alt_fixed_mode,
struct drm_display_mode *downclock_mode)
{
intel_panel_init_backlight_funcs(panel);
panel->fixed_mode = fixed_mode;
+ panel->alt_fixed_mode = alt_fixed_mode;
panel->downclock_mode = downclock_mode;
return 0;
@@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
if (panel->fixed_mode)
drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
+ if (panel->alt_fixed_mode)
+ drm_mode_destroy(intel_connector->base.dev,
+ panel->alt_fixed_mode);
+
if (panel->downclock_mode)
drm_mode_destroy(intel_connector->base.dev,
panel->downclock_mode);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/edp: Allow alternate fixed mode for eDP if available.
2017-08-09 19:48 [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available Jim Bride
@ 2017-08-09 20:14 ` Patchwork
2017-08-10 13:13 ` [PATCH v6] " Jani Nikula
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-08-09 20:14 UTC (permalink / raw)
To: Jim Bride; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/edp: Allow alternate fixed mode for eDP if available.
URL : https://patchwork.freedesktop.org/series/28578/
State : success
== Summary ==
Series 28578v1 drm/i915/edp: Allow alternate fixed mode for eDP if available.
https://patchwork.freedesktop.org/api/1.0/series/28578/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail -> PASS (fi-snb-2600) fdo#100007
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:439s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:424s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:356s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:489s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:491s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s
fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:509s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:588s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:426s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:408s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:419s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:496s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:459s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:579s
fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:574s
fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:528s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:449s
fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:639s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:465s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:424s
fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:484s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:547s
fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:406s
2d0288b5b28c0d67460f0258a41bb4f78b812f29 drm-tip: 2017y-08m-09d-18h-09m-54s UTC integration manifest
13011d53221c drm/i915/edp: Allow alternate fixed mode for eDP if available.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5356/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.
2017-08-09 19:48 [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available Jim Bride
2017-08-09 20:14 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-08-10 13:13 ` Jani Nikula
2017-08-10 16:37 ` Jim Bride
1 sibling, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2017-08-10 13:13 UTC (permalink / raw)
To: Jim Bride, intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
On Wed, 09 Aug 2017, Jim Bride <jim.bride@linux.intel.com> wrote:
> Some fixed resolution panels actually support more than one mode,
> with the only thing different being the refresh rate. Having this
> alternate mode available to us is desirable, because it allows us to
> test PSR on panels whose setup time at the preferred mode is too long.
> With this patch we allow the use of the alternate mode if it's
> available and it was specifically requested.
>
> v2 and v3: Rebase
> v4: * Fix up some leaky mode stuff (Chris)
> * Rebase
> v5: * Fix a NULL pointer derefrence (David Weinehall)
> v6: * Whitespace / spelling / checkpatch clean-up; no functional
> change. (David)
> * Rebase
>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
> drivers/gpu/drm/i915/intel_panel.c | 6 ++++++
> 6 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 76c8a0b..576b5af 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1606,6 +1606,23 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> return bpp;
> }
>
> +static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
> + struct drm_display_mode *m2)
> +{
> + bool bres = false;
What is bres?
> +
> + if (m1 && m2)
> + bres = (m1->hdisplay == m2->hdisplay &&
> + m1->hsync_start == m2->hsync_start &&
> + m1->hsync_end == m2->hsync_end &&
> + m1->htotal == m2->htotal &&
> + m1->vdisplay == m2->vdisplay &&
> + m1->vsync_start == m2->vsync_start &&
> + m1->vsync_end == m2->vsync_end &&
> + m1->vtotal == m2->vtotal);
> + return bres;
You know you could just do
return m1 && m2 && m1->hdisplay == m2->hdisplay && ...
But I'm nitpicking.
BR,
Jani.
> +}
> +
> bool
> intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> @@ -1652,8 +1669,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
>
> if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> - intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> - adjusted_mode);
> + struct drm_display_mode *panel_mode =
> + intel_connector->panel.alt_fixed_mode;
> + struct drm_display_mode *req_mode = &pipe_config->base.mode;
> +
> + if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> + panel_mode = intel_connector->panel.fixed_mode;
> +
> + drm_mode_debug_printmodeline(panel_mode);
> +
> + intel_fixed_panel_mode(panel_mode, adjusted_mode);
>
> if (INTEL_GEN(dev_priv) >= 9) {
> int ret;
> @@ -5780,6 +5805,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_display_mode *fixed_mode = NULL;
> + struct drm_display_mode *alt_fixed_mode = NULL;
> struct drm_display_mode *downclock_mode = NULL;
> bool has_dpcd;
> struct drm_display_mode *scan;
> @@ -5835,13 +5861,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> }
> intel_connector->edid = edid;
>
> - /* prefer fixed mode from EDID if available */
> + /* prefer fixed mode from EDID if available, save an alt mode also */
> list_for_each_entry(scan, &connector->probed_modes, head) {
> if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> fixed_mode = drm_mode_duplicate(dev, scan);
> downclock_mode = intel_dp_drrs_init(
> intel_connector, fixed_mode);
> - break;
> + } else if (!alt_fixed_mode) {
> + alt_fixed_mode = drm_mode_duplicate(dev, scan);
> }
> }
>
> @@ -5878,7 +5905,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> pipe_name(pipe));
> }
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> + intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
> + downclock_mode);
> intel_connector->panel.backlight.power = intel_edp_backlight_power;
> intel_panel_setup_backlight(connector, pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f91de9c..b3fcbb9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -265,6 +265,7 @@ struct intel_encoder {
>
> struct intel_panel {
> struct drm_display_mode *fixed_mode;
> + struct drm_display_mode *alt_fixed_mode;
> struct drm_display_mode *downclock_mode;
>
> /* backlight */
> @@ -1679,6 +1680,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
> /* intel_panel.c */
> int intel_panel_init(struct intel_panel *panel,
> struct drm_display_mode *fixed_mode,
> + struct drm_display_mode *alt_fixed_mode,
> struct drm_display_mode *downclock_mode);
> void intel_panel_fini(struct intel_panel *panel);
> void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index b0b3adf..f0c11ae 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1849,7 +1849,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> connector->display_info.width_mm = fixed_mode->width_mm;
> connector->display_info.height_mm = fixed_mode->height_mm;
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
> intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> intel_dsi_add_properties(intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index baf369d..c0a0272 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -552,7 +552,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> */
> intel_panel_init(&intel_connector->panel,
> intel_dvo_get_current_mode(connector),
> - NULL);
> + 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 61d5579..8e21577 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1138,7 +1138,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> out:
> mutex_unlock(&dev->mode_config.mutex);
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> + intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
> + downclock_mode);
> intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 88018fc..a17b1de 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1920,11 +1920,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>
> int intel_panel_init(struct intel_panel *panel,
> struct drm_display_mode *fixed_mode,
> + struct drm_display_mode *alt_fixed_mode,
> struct drm_display_mode *downclock_mode)
> {
> intel_panel_init_backlight_funcs(panel);
>
> panel->fixed_mode = fixed_mode;
> + panel->alt_fixed_mode = alt_fixed_mode;
> panel->downclock_mode = downclock_mode;
>
> return 0;
> @@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
> if (panel->fixed_mode)
> drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
>
> + if (panel->alt_fixed_mode)
> + drm_mode_destroy(intel_connector->base.dev,
> + panel->alt_fixed_mode);
> +
> if (panel->downclock_mode)
> drm_mode_destroy(intel_connector->base.dev,
> panel->downclock_mode);
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.
2017-08-10 13:13 ` [PATCH v6] " Jani Nikula
@ 2017-08-10 16:37 ` Jim Bride
2017-08-10 17:01 ` Vivi, Rodrigo
0 siblings, 1 reply; 6+ messages in thread
From: Jim Bride @ 2017-08-10 16:37 UTC (permalink / raw)
To: Jani Nikula; +Cc: Paulo Zanoni, intel-gfx, Rodrigo Vivi
On Thu, Aug 10, 2017 at 04:13:44PM +0300, Jani Nikula wrote:
> On Wed, 09 Aug 2017, Jim Bride <jim.bride@linux.intel.com> wrote:
> > Some fixed resolution panels actually support more than one mode,
> > with the only thing different being the refresh rate. Having this
> > alternate mode available to us is desirable, because it allows us to
> > test PSR on panels whose setup time at the preferred mode is too long.
> > With this patch we allow the use of the alternate mode if it's
> > available and it was specifically requested.
> >
> > v2 and v3: Rebase
> > v4: * Fix up some leaky mode stuff (Chris)
> > * Rebase
> > v5: * Fix a NULL pointer derefrence (David Weinehall)
> > v6: * Whitespace / spelling / checkpatch clean-up; no functional
> > change. (David)
> > * Rebase
> >
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++-----
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> > drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> > drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
> > drivers/gpu/drm/i915/intel_panel.c | 6 ++++++
> > 6 files changed, 45 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 76c8a0b..576b5af 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1606,6 +1606,23 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> > return bpp;
> > }
> >
> > +static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
> > + struct drm_display_mode *m2)
> > +{
> > + bool bres = false;
>
> What is bres?
boolean result :)
> > +
> > + if (m1 && m2)
> > + bres = (m1->hdisplay == m2->hdisplay &&
> > + m1->hsync_start == m2->hsync_start &&
> > + m1->hsync_end == m2->hsync_end &&
> > + m1->htotal == m2->htotal &&
> > + m1->vdisplay == m2->vdisplay &&
> > + m1->vsync_start == m2->vsync_start &&
> > + m1->vsync_end == m2->vsync_end &&
> > + m1->vtotal == m2->vtotal);
> > + return bres;
>
> You know you could just do
>
> return m1 && m2 && m1->hdisplay == m2->hdisplay && ...
Yeah, but I think the above is a bit more readable, and also makes
it easier to add debug instrumentation at some point if needed.
Jim
> But I'm nitpicking.
>
> BR,
> Jani.
>
>
> > +}
> > +
> > bool
> > intel_dp_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config,
> > @@ -1652,8 +1669,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
> >
> > if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> > - intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> > - adjusted_mode);
> > + struct drm_display_mode *panel_mode =
> > + intel_connector->panel.alt_fixed_mode;
> > + struct drm_display_mode *req_mode = &pipe_config->base.mode;
> > +
> > + if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> > + panel_mode = intel_connector->panel.fixed_mode;
> > +
> > + drm_mode_debug_printmodeline(panel_mode);
> > +
> > + intel_fixed_panel_mode(panel_mode, adjusted_mode);
> >
> > if (INTEL_GEN(dev_priv) >= 9) {
> > int ret;
> > @@ -5780,6 +5805,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > struct drm_device *dev = intel_encoder->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > struct drm_display_mode *fixed_mode = NULL;
> > + struct drm_display_mode *alt_fixed_mode = NULL;
> > struct drm_display_mode *downclock_mode = NULL;
> > bool has_dpcd;
> > struct drm_display_mode *scan;
> > @@ -5835,13 +5861,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > }
> > intel_connector->edid = edid;
> >
> > - /* prefer fixed mode from EDID if available */
> > + /* prefer fixed mode from EDID if available, save an alt mode also */
> > list_for_each_entry(scan, &connector->probed_modes, head) {
> > if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> > fixed_mode = drm_mode_duplicate(dev, scan);
> > downclock_mode = intel_dp_drrs_init(
> > intel_connector, fixed_mode);
> > - break;
> > + } else if (!alt_fixed_mode) {
> > + alt_fixed_mode = drm_mode_duplicate(dev, scan);
> > }
> > }
> >
> > @@ -5878,7 +5905,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > pipe_name(pipe));
> > }
> >
> > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> > + intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
> > + downclock_mode);
> > intel_connector->panel.backlight.power = intel_edp_backlight_power;
> > intel_panel_setup_backlight(connector, pipe);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index f91de9c..b3fcbb9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -265,6 +265,7 @@ struct intel_encoder {
> >
> > struct intel_panel {
> > struct drm_display_mode *fixed_mode;
> > + struct drm_display_mode *alt_fixed_mode;
> > struct drm_display_mode *downclock_mode;
> >
> > /* backlight */
> > @@ -1679,6 +1680,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
> > /* intel_panel.c */
> > int intel_panel_init(struct intel_panel *panel,
> > struct drm_display_mode *fixed_mode,
> > + struct drm_display_mode *alt_fixed_mode,
> > struct drm_display_mode *downclock_mode);
> > void intel_panel_fini(struct intel_panel *panel);
> > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index b0b3adf..f0c11ae 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1849,7 +1849,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> > connector->display_info.width_mm = fixed_mode->width_mm;
> > connector->display_info.height_mm = fixed_mode->height_mm;
> >
> > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
> > intel_panel_setup_backlight(connector, INVALID_PIPE);
> >
> > intel_dsi_add_properties(intel_connector);
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index baf369d..c0a0272 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -552,7 +552,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> > */
> > intel_panel_init(&intel_connector->panel,
> > intel_dvo_get_current_mode(connector),
> > - NULL);
> > + 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 61d5579..8e21577 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -1138,7 +1138,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > out:
> > mutex_unlock(&dev->mode_config.mutex);
> >
> > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
> > + downclock_mode);
> > intel_panel_setup_backlight(connector, INVALID_PIPE);
> >
> > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 88018fc..a17b1de 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1920,11 +1920,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> >
> > int intel_panel_init(struct intel_panel *panel,
> > struct drm_display_mode *fixed_mode,
> > + struct drm_display_mode *alt_fixed_mode,
> > struct drm_display_mode *downclock_mode)
> > {
> > intel_panel_init_backlight_funcs(panel);
> >
> > panel->fixed_mode = fixed_mode;
> > + panel->alt_fixed_mode = alt_fixed_mode;
> > panel->downclock_mode = downclock_mode;
> >
> > return 0;
> > @@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
> > if (panel->fixed_mode)
> > drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
> >
> > + if (panel->alt_fixed_mode)
> > + drm_mode_destroy(intel_connector->base.dev,
> > + panel->alt_fixed_mode);
> > +
> > if (panel->downclock_mode)
> > drm_mode_destroy(intel_connector->base.dev,
> > panel->downclock_mode);
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.
2017-08-10 16:37 ` Jim Bride
@ 2017-08-10 17:01 ` Vivi, Rodrigo
2017-08-15 23:46 ` Rodrigo Vivi
0 siblings, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2017-08-10 17:01 UTC (permalink / raw)
To: jim.bride@linux.intel.com
Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, Zanoni, Paulo R
On Thu, 2017-08-10 at 09:37 -0700, Jim Bride wrote:
> On Thu, Aug 10, 2017 at 04:13:44PM +0300, Jani Nikula wrote:
> > On Wed, 09 Aug 2017, Jim Bride <jim.bride@linux.intel.com> wrote:
> > > Some fixed resolution panels actually support more than one mode,
> > > with the only thing different being the refresh rate. Having this
> > > alternate mode available to us is desirable, because it allows us to
> > > test PSR on panels whose setup time at the preferred mode is too long.
> > > With this patch we allow the use of the alternate mode if it's
> > > available and it was specifically requested.
> > >
> > > v2 and v3: Rebase
> > > v4: * Fix up some leaky mode stuff (Chris)
> > > * Rebase
> > > v5: * Fix a NULL pointer derefrence (David Weinehall)
> > > v6: * Whitespace / spelling / checkpatch clean-up; no functional
> > > change. (David)
> > > * Rebase
> > >
> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
> > > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++-----
> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> > > drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> > > drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
> > > drivers/gpu/drm/i915/intel_panel.c | 6 ++++++
> > > 6 files changed, 45 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 76c8a0b..576b5af 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1606,6 +1606,23 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> > > return bpp;
> > > }
> > >
> > > +static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
> > > + struct drm_display_mode *m2)
> > > +{
> > > + bool bres = false;
> >
> > What is bres?
>
> boolean result :)
maybe something less mysterious and more meaningful would be better like
bool mode_match;
>
> > > +
> > > + if (m1 && m2)
> > > + bres = (m1->hdisplay == m2->hdisplay &&
> > > + m1->hsync_start == m2->hsync_start &&
> > > + m1->hsync_end == m2->hsync_end &&
> > > + m1->htotal == m2->htotal &&
> > > + m1->vdisplay == m2->vdisplay &&
> > > + m1->vsync_start == m2->vsync_start &&
> > > + m1->vsync_end == m2->vsync_end &&
> > > + m1->vtotal == m2->vtotal);
> > > + return bres;
> >
> > You know you could just do
> >
> > return m1 && m2 && m1->hdisplay == m2->hdisplay && ...
>
> Yeah, but I think the above is a bit more readable, and also makes
> it easier to add debug instrumentation at some point if needed.
... anyways I believe the direct return is easier to read than "bres"
and also if some debug instrumentation is needed later it can be
modified later.
>
> Jim
>
> > But I'm nitpicking.
me too...
so if this is the final version and nobody complains deeply and it is
already reviewed I intend to merge soon... please advise otherwise...
> >
> > BR,
> > Jani.
> >
> >
> > > +}
> > > +
> > > bool
> > > intel_dp_compute_config(struct intel_encoder *encoder,
> > > struct intel_crtc_state *pipe_config,
> > > @@ -1652,8 +1669,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > > pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
> > >
> > > if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> > > - intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> > > - adjusted_mode);
> > > + struct drm_display_mode *panel_mode =
> > > + intel_connector->panel.alt_fixed_mode;
> > > + struct drm_display_mode *req_mode = &pipe_config->base.mode;
> > > +
> > > + if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> > > + panel_mode = intel_connector->panel.fixed_mode;
> > > +
> > > + drm_mode_debug_printmodeline(panel_mode);
> > > +
> > > + intel_fixed_panel_mode(panel_mode, adjusted_mode);
> > >
> > > if (INTEL_GEN(dev_priv) >= 9) {
> > > int ret;
> > > @@ -5780,6 +5805,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > > struct drm_device *dev = intel_encoder->base.dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > struct drm_display_mode *fixed_mode = NULL;
> > > + struct drm_display_mode *alt_fixed_mode = NULL;
> > > struct drm_display_mode *downclock_mode = NULL;
> > > bool has_dpcd;
> > > struct drm_display_mode *scan;
> > > @@ -5835,13 +5861,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > > }
> > > intel_connector->edid = edid;
> > >
> > > - /* prefer fixed mode from EDID if available */
> > > + /* prefer fixed mode from EDID if available, save an alt mode also */
> > > list_for_each_entry(scan, &connector->probed_modes, head) {
> > > if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> > > fixed_mode = drm_mode_duplicate(dev, scan);
> > > downclock_mode = intel_dp_drrs_init(
> > > intel_connector, fixed_mode);
> > > - break;
> > > + } else if (!alt_fixed_mode) {
> > > + alt_fixed_mode = drm_mode_duplicate(dev, scan);
> > > }
> > > }
> > >
> > > @@ -5878,7 +5905,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > > pipe_name(pipe));
> > > }
> > >
> > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> > > + intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
> > > + downclock_mode);
> > > intel_connector->panel.backlight.power = intel_edp_backlight_power;
> > > intel_panel_setup_backlight(connector, pipe);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index f91de9c..b3fcbb9 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -265,6 +265,7 @@ struct intel_encoder {
> > >
> > > struct intel_panel {
> > > struct drm_display_mode *fixed_mode;
> > > + struct drm_display_mode *alt_fixed_mode;
> > > struct drm_display_mode *downclock_mode;
> > >
> > > /* backlight */
> > > @@ -1679,6 +1680,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
> > > /* intel_panel.c */
> > > int intel_panel_init(struct intel_panel *panel,
> > > struct drm_display_mode *fixed_mode,
> > > + struct drm_display_mode *alt_fixed_mode,
> > > struct drm_display_mode *downclock_mode);
> > > void intel_panel_fini(struct intel_panel *panel);
> > > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > > index b0b3adf..f0c11ae 100644
> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > @@ -1849,7 +1849,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> > > connector->display_info.width_mm = fixed_mode->width_mm;
> > > connector->display_info.height_mm = fixed_mode->height_mm;
> > >
> > > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> > > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
> > > intel_panel_setup_backlight(connector, INVALID_PIPE);
> > >
> > > intel_dsi_add_properties(intel_connector);
> > > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > > index baf369d..c0a0272 100644
> > > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > > @@ -552,7 +552,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> > > */
> > > intel_panel_init(&intel_connector->panel,
> > > intel_dvo_get_current_mode(connector),
> > > - NULL);
> > > + 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 61d5579..8e21577 100644
> > > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > > @@ -1138,7 +1138,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> > > out:
> > > mutex_unlock(&dev->mode_config.mutex);
> > >
> > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> > > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
> > > + downclock_mode);
> > > intel_panel_setup_backlight(connector, INVALID_PIPE);
> > >
> > > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > > index 88018fc..a17b1de 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -1920,11 +1920,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> > >
> > > int intel_panel_init(struct intel_panel *panel,
> > > struct drm_display_mode *fixed_mode,
> > > + struct drm_display_mode *alt_fixed_mode,
> > > struct drm_display_mode *downclock_mode)
> > > {
> > > intel_panel_init_backlight_funcs(panel);
> > >
> > > panel->fixed_mode = fixed_mode;
> > > + panel->alt_fixed_mode = alt_fixed_mode;
> > > panel->downclock_mode = downclock_mode;
> > >
> > > return 0;
> > > @@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
> > > if (panel->fixed_mode)
> > > drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
> > >
> > > + if (panel->alt_fixed_mode)
> > > + drm_mode_destroy(intel_connector->base.dev,
> > > + panel->alt_fixed_mode);
> > > +
> > > if (panel->downclock_mode)
> > > drm_mode_destroy(intel_connector->base.dev,
> > > panel->downclock_mode);
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.
2017-08-10 17:01 ` Vivi, Rodrigo
@ 2017-08-15 23:46 ` Rodrigo Vivi
0 siblings, 0 replies; 6+ messages in thread
From: Rodrigo Vivi @ 2017-08-15 23:46 UTC (permalink / raw)
To: Vivi, Rodrigo
Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, Zanoni, Paulo R
On Thu, Aug 10, 2017 at 10:01 AM, Vivi, Rodrigo <rodrigo.vivi@intel.com> wrote:
> On Thu, 2017-08-10 at 09:37 -0700, Jim Bride wrote:
>> On Thu, Aug 10, 2017 at 04:13:44PM +0300, Jani Nikula wrote:
>> > On Wed, 09 Aug 2017, Jim Bride <jim.bride@linux.intel.com> wrote:
>> > > Some fixed resolution panels actually support more than one mode,
>> > > with the only thing different being the refresh rate. Having this
>> > > alternate mode available to us is desirable, because it allows us to
>> > > test PSR on panels whose setup time at the preferred mode is too long.
>> > > With this patch we allow the use of the alternate mode if it's
>> > > available and it was specifically requested.
>> > >
>> > > v2 and v3: Rebase
>> > > v4: * Fix up some leaky mode stuff (Chris)
>> > > * Rebase
>> > > v5: * Fix a NULL pointer derefrence (David Weinehall)
>> > > v6: * Whitespace / spelling / checkpatch clean-up; no functional
>> > > change. (David)
>> > > * Rebase
>> > >
>> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
>> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > > Cc: Jani Nikula <jani.nikula@intel.com>
>> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
>> > > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
>> > > ---
>> > > drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++-----
>> > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
>> > > drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>> > > drivers/gpu/drm/i915/intel_dvo.c | 2 +-
>> > > drivers/gpu/drm/i915/intel_lvds.c | 3 ++-
>> > > drivers/gpu/drm/i915/intel_panel.c | 6 ++++++
>> > > 6 files changed, 45 insertions(+), 8 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > > index 76c8a0b..576b5af 100644
>> > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > @@ -1606,6 +1606,23 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>> > > return bpp;
>> > > }
>> > >
>> > > +static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
>> > > + struct drm_display_mode *m2)
>> > > +{
>> > > + bool bres = false;
>> >
>> > What is bres?
>>
>> boolean result :)
>
> maybe something less mysterious and more meaningful would be better like
> bool mode_match;
>
>>
>> > > +
>> > > + if (m1 && m2)
>> > > + bres = (m1->hdisplay == m2->hdisplay &&
>> > > + m1->hsync_start == m2->hsync_start &&
>> > > + m1->hsync_end == m2->hsync_end &&
>> > > + m1->htotal == m2->htotal &&
>> > > + m1->vdisplay == m2->vdisplay &&
>> > > + m1->vsync_start == m2->vsync_start &&
>> > > + m1->vsync_end == m2->vsync_end &&
>> > > + m1->vtotal == m2->vtotal);
>> > > + return bres;
>> >
>> > You know you could just do
>> >
>> > return m1 && m2 && m1->hdisplay == m2->hdisplay && ...
>>
>> Yeah, but I think the above is a bit more readable, and also makes
>> it easier to add debug instrumentation at some point if needed.
>
> ... anyways I believe the direct return is easier to read than "bres"
> and also if some debug instrumentation is needed later it can be
> modified later.
>
>>
>> Jim
>>
>> > But I'm nitpicking.
>
> me too...
>
> so if this is the final version and nobody complains deeply and it is
> already reviewed I intend to merge soon... please advise otherwise...
patch merged to dinq. thanks for patches, reviews and tests.
>
>> >
>> > BR,
>> > Jani.
>> >
>> >
>> > > +}
>> > > +
>> > > bool
>> > > intel_dp_compute_config(struct intel_encoder *encoder,
>> > > struct intel_crtc_state *pipe_config,
>> > > @@ -1652,8 +1669,16 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>> > > pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
>> > >
>> > > if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
>> > > - intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
>> > > - adjusted_mode);
>> > > + struct drm_display_mode *panel_mode =
>> > > + intel_connector->panel.alt_fixed_mode;
>> > > + struct drm_display_mode *req_mode = &pipe_config->base.mode;
>> > > +
>> > > + if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
>> > > + panel_mode = intel_connector->panel.fixed_mode;
>> > > +
>> > > + drm_mode_debug_printmodeline(panel_mode);
>> > > +
>> > > + intel_fixed_panel_mode(panel_mode, adjusted_mode);
>> > >
>> > > if (INTEL_GEN(dev_priv) >= 9) {
>> > > int ret;
>> > > @@ -5780,6 +5805,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> > > struct drm_device *dev = intel_encoder->base.dev;
>> > > struct drm_i915_private *dev_priv = to_i915(dev);
>> > > struct drm_display_mode *fixed_mode = NULL;
>> > > + struct drm_display_mode *alt_fixed_mode = NULL;
>> > > struct drm_display_mode *downclock_mode = NULL;
>> > > bool has_dpcd;
>> > > struct drm_display_mode *scan;
>> > > @@ -5835,13 +5861,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> > > }
>> > > intel_connector->edid = edid;
>> > >
>> > > - /* prefer fixed mode from EDID if available */
>> > > + /* prefer fixed mode from EDID if available, save an alt mode also */
>> > > list_for_each_entry(scan, &connector->probed_modes, head) {
>> > > if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
>> > > fixed_mode = drm_mode_duplicate(dev, scan);
>> > > downclock_mode = intel_dp_drrs_init(
>> > > intel_connector, fixed_mode);
>> > > - break;
>> > > + } else if (!alt_fixed_mode) {
>> > > + alt_fixed_mode = drm_mode_duplicate(dev, scan);
>> > > }
>> > > }
>> > >
>> > > @@ -5878,7 +5905,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> > > pipe_name(pipe));
>> > > }
>> > >
>> > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>> > > + intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
>> > > + downclock_mode);
>> > > intel_connector->panel.backlight.power = intel_edp_backlight_power;
>> > > intel_panel_setup_backlight(connector, pipe);
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > > index f91de9c..b3fcbb9 100644
>> > > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > > @@ -265,6 +265,7 @@ struct intel_encoder {
>> > >
>> > > struct intel_panel {
>> > > struct drm_display_mode *fixed_mode;
>> > > + struct drm_display_mode *alt_fixed_mode;
>> > > struct drm_display_mode *downclock_mode;
>> > >
>> > > /* backlight */
>> > > @@ -1679,6 +1680,7 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
>> > > /* intel_panel.c */
>> > > int intel_panel_init(struct intel_panel *panel,
>> > > struct drm_display_mode *fixed_mode,
>> > > + struct drm_display_mode *alt_fixed_mode,
>> > > struct drm_display_mode *downclock_mode);
>> > > void intel_panel_fini(struct intel_panel *panel);
>> > > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> > > index b0b3adf..f0c11ae 100644
>> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > > @@ -1849,7 +1849,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
>> > > connector->display_info.width_mm = fixed_mode->width_mm;
>> > > connector->display_info.height_mm = fixed_mode->height_mm;
>> > >
>> > > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>> > > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
>> > > intel_panel_setup_backlight(connector, INVALID_PIPE);
>> > >
>> > > intel_dsi_add_properties(intel_connector);
>> > > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
>> > > index baf369d..c0a0272 100644
>> > > --- a/drivers/gpu/drm/i915/intel_dvo.c
>> > > +++ b/drivers/gpu/drm/i915/intel_dvo.c
>> > > @@ -552,7 +552,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
>> > > */
>> > > intel_panel_init(&intel_connector->panel,
>> > > intel_dvo_get_current_mode(connector),
>> > > - NULL);
>> > > + 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 61d5579..8e21577 100644
>> > > --- a/drivers/gpu/drm/i915/intel_lvds.c
>> > > +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> > > @@ -1138,7 +1138,8 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>> > > out:
>> > > mutex_unlock(&dev->mode_config.mutex);
>> > >
>> > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>> > > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
>> > > + downclock_mode);
>> > > intel_panel_setup_backlight(connector, INVALID_PIPE);
>> > >
>> > > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
>> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> > > index 88018fc..a17b1de 100644
>> > > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > > @@ -1920,11 +1920,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>> > >
>> > > int intel_panel_init(struct intel_panel *panel,
>> > > struct drm_display_mode *fixed_mode,
>> > > + struct drm_display_mode *alt_fixed_mode,
>> > > struct drm_display_mode *downclock_mode)
>> > > {
>> > > intel_panel_init_backlight_funcs(panel);
>> > >
>> > > panel->fixed_mode = fixed_mode;
>> > > + panel->alt_fixed_mode = alt_fixed_mode;
>> > > panel->downclock_mode = downclock_mode;
>> > >
>> > > return 0;
>> > > @@ -1938,6 +1940,10 @@ void intel_panel_fini(struct intel_panel *panel)
>> > > if (panel->fixed_mode)
>> > > drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
>> > >
>> > > + if (panel->alt_fixed_mode)
>> > > + drm_mode_destroy(intel_connector->base.dev,
>> > > + panel->alt_fixed_mode);
>> > > +
>> > > if (panel->downclock_mode)
>> > > drm_mode_destroy(intel_connector->base.dev,
>> > > panel->downclock_mode);
>> >
>> > --
>> > Jani Nikula, Intel Open Source Technology Center
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-15 23:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-09 19:48 [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available Jim Bride
2017-08-09 20:14 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-10 13:13 ` [PATCH v6] " Jani Nikula
2017-08-10 16:37 ` Jim Bride
2017-08-10 17:01 ` Vivi, Rodrigo
2017-08-15 23:46 ` Rodrigo Vivi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox