Re: [PATCH] drm/i915/skl: Add module parameter to select edp vswing table

Jani Nikula
2015-05-05T11:37:45Z

On Tue, 05 May 2015, Sonika Jindal <sonika.jindal@intel.com> wrote:
> This provides an option to override the value set by VBT
> for selecting edp Vswing Pre-emph setting table.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=89554
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 +++-
> drivers/gpu/drm/i915/i915_params.c | 7 +++++++
> drivers/gpu/drm/i915/intel_bios.c | 9 +++++++--
> drivers/gpu/drm/i915/intel_ddi.c | 2 +-
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> 5 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 136d42a..1c3ee66 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1364,7 +1364,6 @@ struct intel_vbt_data {
> bool edp_initialized;
> bool edp_support;
> int edp_bpp;
> - bool edp_low_vswing;
> struct edp_power_seq edp_pps;
>
> struct {
> @@ -1846,6 +1845,8 @@ struct drm_i915_private {
> void (*stop_ring)(struct intel_engine_cs *ring);
> } gt;
>
> + bool edp_low_vswing;
> +
> /*
> * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> * will be rejected. Instead look for a better place.
> @@ -2515,6 +2516,7 @@ struct i915_params {
> int mmio_debug;
> bool verbose_state_checks;
> bool nuclear_pageflip;
> + int edp_vswing;
> };
> extern struct i915_params i915 __read_mostly;
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index bb64415..d245ac5 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -53,6 +53,7 @@ struct i915_params i915 __read_mostly = {
> .mmio_debug = 0,
> .verbose_state_checks = 1,
> .nuclear_pageflip = 0,
> + .edp_vswing = 0,
> };
>
> module_param_named(modeset, i915.modeset, int, 0400);
> @@ -184,3 +185,9 @@ MODULE_PARM_DESC(verbose_state_checks,
> module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600);
> MODULE_PARM_DESC(nuclear_pageflip,
> "Force atomic modeset functionality; only planes work for now (default: false).");
> +
> +module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0600);
> +MODULE_PARM_DESC(edp_vswing,
> + "Ignore/Override vswing pre-emph table selection from VBT "
> + "(0=use value from vbt [default], 1=low power swing(200mV),"
> + "2=default swing(400mV))");
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index c08368c..cee596d 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -672,8 +672,13 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
> if (bdb->version >= 173) {
> uint8_t vswing;
>
> - vswing = (edp->edp_vswing_preemph >> (panel_type * 4)) & 0xF;
> - dev_priv->vbt.edp_low_vswing = vswing == 0;
> + /* Don't read from VBT if module parameter has valid value*/
> + if (i915.edp_vswing) {
> + dev_priv->edp_low_vswing = i915.edp_vswing == 1;
> + } else {
> + vswing = (edp->edp_vswing_preemph >> (panel_type * 4)) & 0xF;
> + dev_priv->edp_low_vswing = vswing == 0;
> + }

Please keep dev_priv->vbt.edp_low_vswing as it is, and check the module
parameter where that is used. If we're adding a module parameter, it
will be immensely useful to be able to change the value at runtime to
check if it makes a difference. What you suggest requires a module
reload.

There's still the question whether we can default to using the vbt value
if that can be broken...

BR,
Jani.

> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 9c1e74a..2539f22 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -282,7 +282,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> ddi_translations_fdi = NULL;
> ddi_translations_dp = skl_ddi_translations_dp;
> n_dp_entries = ARRAY_SIZE(skl_ddi_translations_dp);
> - if (dev_priv->vbt.edp_low_vswing) {
> + if (dev_priv->edp_low_vswing) {
> ddi_translations_edp = skl_ddi_translations_edp;
> n_edp_entries = ARRAY_SIZE(skl_ddi_translations_edp);
> } else {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bacdec5..5f53666 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2894,7 +2894,7 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
> if (IS_BROXTON(dev))
> return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> else if (INTEL_INFO(dev)->gen >= 9) {
> - if (dev_priv->vbt.edp_low_vswing && port == PORT_A)
> + if (dev_priv->edp_low_vswing && port == PORT_A)
> return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
> } else if (IS_VALLEYVIEW(dev))
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Jani Nikula, Intel Open Source Technology Center