* [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles
@ 2017-11-22 18:55 Lucas De Marchi
2017-11-22 19:45 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Lucas De Marchi @ 2017-11-22 18:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Arthur J Runyan, Lucas De Marchi, Rodrigo Vivi
WA Display #1178 is meant to fix Aux channel voltage swing too low with
some type C dongles. Although it is for type C, HW engineers reported
that it can be applied to all external ports even if they are not going
to type C.
For CNL we apply the workaround every time Aux B, C and D are powering
up since they will lose the configuration when powered down.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Arthur J Runyan <arthur.j.runyan@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
Since this is a workaround I think it would be desirable not to be
so intrusive. The simplest thing to do is to add the
IS_CANNONLAKE() and workaround as done here.
An alternative that may be more elegant (but also more intrusive) is to
declare a new ops for CNL for AUX B/C/D. Let me know what you think.
For the type-C dongles that I have here it worked both with and without
this patch, so bear in mind I couldn't actually reproduce the problem.
drivers/gpu/drm/i915/i915_reg.h | 11 +++++++++++
drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 96c80fa0fcac..32064605f82d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8332,6 +8332,17 @@ enum skl_power_gate {
#define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1)
#define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg)))
+#define _CNL_AUX_REG_IDX(pw) ((pw - 1) >> 4)
+#define _CNL_AUX_ANAOVRD1_B 0x162250
+#define _CNL_AUX_ANAOVRD1_C 0x162210
+#define _CNL_AUX_ANAOVRD1_D 0x1622D0
+#define CNL_AUX_ANAOVRD1(pw) _MMIO(_PICK(_CNL_AUX_REG_IDX(pw), \
+ _CNL_AUX_ANAOVRD1_B, \
+ _CNL_AUX_ANAOVRD1_C, \
+ _CNL_AUX_ANAOVRD1_D))
+#define CNL_AUX_ANAOVRD1_ENABLE (1<<16)
+#define CNL_AUX_ANAOVRD1_LDO_BYPASS (1<<23)
+
/* Per-pipe DDI Function Control */
#define _TRANS_DDI_FUNC_CTL_A 0x60400
#define _TRANS_DDI_FUNC_CTL_B 0x61400
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8315499452dc..9bf200e4885d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -388,6 +388,15 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), val | HSW_PWR_WELL_CTL_REQ(id));
hsw_wait_for_power_well_enable(dev_priv, power_well);
+ /* WA Display #1178 */
+ if (IS_CANNONLAKE(dev_priv) &&
+ (id == CNL_DISP_PW_AUX_B || id == CNL_DISP_PW_AUX_C ||
+ id == CNL_DISP_PW_AUX_D)) {
+ val = I915_READ(CNL_AUX_ANAOVRD1(id));
+ val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
+ I915_WRITE(CNL_AUX_ANAOVRD1(id), val);
+ }
+
if (wait_fuses)
gen9_wait_for_power_well_fuses(dev_priv, pg);
--
2.14.3
_______________________________________________
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/cnl: WA Display #1178 to fix some type C dongles
2017-11-22 18:55 [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles Lucas De Marchi
@ 2017-11-22 19:45 ` Patchwork
2017-11-22 21:39 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-11-23 15:21 ` [PATCH] " Ville Syrjälä
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-11-22 19:45 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/cnl: WA Display #1178 to fix some type C dongles
URL : https://patchwork.freedesktop.org/series/34248/
State : success
== Summary ==
Series 34248v1 drm/i915/cnl: WA Display #1178 to fix some type C dongles
https://patchwork.freedesktop.org/api/1.0/series/34248/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass -> FAIL (fi-skl-6700k) fdo#103191
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:446s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:464s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:384s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:530s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:509s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:497s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:489s
fi-cfl-s2 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:609s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:429s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:266s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:539s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:442s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:428s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:488s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:462s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:480s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:537s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:474s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:536s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:578s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:544s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:564s
fi-skl-6700k total:289 pass:264 dwarn:0 dfail:0 fail:1 skip:24 time:516s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:495s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:465s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:562s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:422s
Blacklisted hosts:
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:572s
fi-glk-dsi total:18 pass:17 dwarn:0 dfail:0 fail:0 skip:0
a3f080ee939e8befca924f1ab83deeabfa7ff98e drm-tip: 2017y-11m-22d-18h-37m-24s UTC integration manifest
e4a04cd85b29 drm/i915/cnl: WA Display #1178 to fix some type C dongles
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7240/
_______________________________________________
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
* ✗ Fi.CI.IGT: warning for drm/i915/cnl: WA Display #1178 to fix some type C dongles
2017-11-22 18:55 [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles Lucas De Marchi
2017-11-22 19:45 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-11-22 21:39 ` Patchwork
2017-11-23 15:21 ` [PATCH] " Ville Syrjälä
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-11-22 21:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/cnl: WA Display #1178 to fix some type C dongles
URL : https://patchwork.freedesktop.org/series/34248/
State : warning
== Summary ==
Test kms_setmode:
Subgroup basic:
fail -> PASS (shard-hsw) fdo#99912
Test kms_cursor_crc:
Subgroup cursor-64x64-onscreen:
pass -> SKIP (shard-hsw)
Test kms_frontbuffer_tracking:
Subgroup fbc-rgb101010-draw-pwrite:
pass -> SKIP (shard-hsw)
Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
pass -> SKIP (shard-hsw)
Subgroup fbc-1p-pri-indfb-multidraw:
incomplete -> PASS (shard-hsw) fdo#103167
Test drv_module_reload:
Subgroup basic-no-display:
pass -> DMESG-WARN (shard-hsw) fdo#102707
Test drv_suspend:
Subgroup fence-restore-tiled2untiled:
pass -> SKIP (shard-snb)
Test gem_busy:
Subgroup close-race:
fail -> PASS (shard-snb) fdo#103829
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#103829 https://bugs.freedesktop.org/show_bug.cgi?id=103829
shard-hsw total:2667 pass:1523 dwarn:2 dfail:0 fail:17 skip:1125 time:9525s
shard-snb total:2667 pass:1306 dwarn:1 dfail:0 fail:18 skip:1342 time:8070s
Blacklisted hosts:
shard-apl total:2593 pass:1641 dwarn:3 dfail:0 fail:21 skip:927 time:13002s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7240/shards.html
_______________________________________________
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] drm/i915/cnl: WA Display #1178 to fix some type C dongles
2017-11-22 18:55 [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles Lucas De Marchi
2017-11-22 19:45 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-11-22 21:39 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2017-11-23 15:21 ` Ville Syrjälä
2017-11-27 23:14 ` Lucas De Marchi
2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2017-11-23 15:21 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx, Arthur J Runyan, Rodrigo Vivi
On Wed, Nov 22, 2017 at 10:55:14AM -0800, Lucas De Marchi wrote:
> WA Display #1178 is meant to fix Aux channel voltage swing too low with
> some type C dongles. Although it is for type C, HW engineers reported
> that it can be applied to all external ports even if they are not going
> to type C.
>
> For CNL we apply the workaround every time Aux B, C and D are powering
> up since they will lose the configuration when powered down.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Arthur J Runyan <arthur.j.runyan@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>
> Since this is a workaround I think it would be desirable not to be
> so intrusive. The simplest thing to do is to add the
> IS_CANNONLAKE() and workaround as done here.
>
> An alternative that may be more elegant (but also more intrusive) is to
> declare a new ops for CNL for AUX B/C/D. Let me know what you think.
>
> For the type-C dongles that I have here it worked both with and without
> this patch, so bear in mind I couldn't actually reproduce the problem.
>
> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++++++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 96c80fa0fcac..32064605f82d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8332,6 +8332,17 @@ enum skl_power_gate {
> #define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1)
> #define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg)))
>
> +#define _CNL_AUX_REG_IDX(pw) ((pw - 1) >> 4)
> +#define _CNL_AUX_ANAOVRD1_B 0x162250
> +#define _CNL_AUX_ANAOVRD1_C 0x162210
> +#define _CNL_AUX_ANAOVRD1_D 0x1622D0
> +#define CNL_AUX_ANAOVRD1(pw) _MMIO(_PICK(_CNL_AUX_REG_IDX(pw), \
> + _CNL_AUX_ANAOVRD1_B, \
> + _CNL_AUX_ANAOVRD1_C, \
> + _CNL_AUX_ANAOVRD1_D))
> +#define CNL_AUX_ANAOVRD1_ENABLE (1<<16)
> +#define CNL_AUX_ANAOVRD1_LDO_BYPASS (1<<23)
I can't actually find these registers in bspec. How do you come up with
the names and stuff?
Based on the offset they look like PHY registers to me, so probably
should be placed somewhere around the existing PHY registers.
> +
> /* Per-pipe DDI Function Control */
> #define _TRANS_DDI_FUNC_CTL_A 0x60400
> #define _TRANS_DDI_FUNC_CTL_B 0x61400
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8315499452dc..9bf200e4885d 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -388,6 +388,15 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
> I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), val | HSW_PWR_WELL_CTL_REQ(id));
> hsw_wait_for_power_well_enable(dev_priv, power_well);
>
> + /* WA Display #1178 */
Pls stick to a consistent w/a comment style.
> + if (IS_CANNONLAKE(dev_priv) &&
> + (id == CNL_DISP_PW_AUX_B || id == CNL_DISP_PW_AUX_C ||
> + id == CNL_DISP_PW_AUX_D)) {
> + val = I915_READ(CNL_AUX_ANAOVRD1(id));
> + val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
> + I915_WRITE(CNL_AUX_ANAOVRD1(id), val);
> + }
> +
> if (wait_fuses)
> gen9_wait_for_power_well_fuses(dev_priv, pg);
>
> --
> 2.14.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 6+ messages in thread
* Re: [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles
2017-11-23 15:21 ` [PATCH] " Ville Syrjälä
@ 2017-11-27 23:14 ` Lucas De Marchi
2017-11-28 14:36 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Lucas De Marchi @ 2017-11-27 23:14 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, Arthur J Runyan, Lucas De Marchi, Rodrigo Vivi
On Thu, Nov 23, 2017 at 7:21 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 22, 2017 at 10:55:14AM -0800, Lucas De Marchi wrote:
>> WA Display #1178 is meant to fix Aux channel voltage swing too low with
>> some type C dongles. Although it is for type C, HW engineers reported
>> that it can be applied to all external ports even if they are not going
>> to type C.
>>
>> For CNL we apply the workaround every time Aux B, C and D are powering
>> up since they will lose the configuration when powered down.
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Arthur J Runyan <arthur.j.runyan@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>
>> Since this is a workaround I think it would be desirable not to be
>> so intrusive. The simplest thing to do is to add the
>> IS_CANNONLAKE() and workaround as done here.
>>
>> An alternative that may be more elegant (but also more intrusive) is to
>> declare a new ops for CNL for AUX B/C/D. Let me know what you think.
>>
>> For the type-C dongles that I have here it worked both with and without
>> this patch, so bear in mind I couldn't actually reproduce the problem.
>>
>> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++++++
>> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++++++++
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 96c80fa0fcac..32064605f82d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8332,6 +8332,17 @@ enum skl_power_gate {
>> #define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1)
>> #define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg)))
>>
>> +#define _CNL_AUX_REG_IDX(pw) ((pw - 1) >> 4)
>> +#define _CNL_AUX_ANAOVRD1_B 0x162250
>> +#define _CNL_AUX_ANAOVRD1_C 0x162210
>> +#define _CNL_AUX_ANAOVRD1_D 0x1622D0
>> +#define CNL_AUX_ANAOVRD1(pw) _MMIO(_PICK(_CNL_AUX_REG_IDX(pw), \
>> + _CNL_AUX_ANAOVRD1_B, \
>> + _CNL_AUX_ANAOVRD1_C, \
>> + _CNL_AUX_ANAOVRD1_D))
>> +#define CNL_AUX_ANAOVRD1_ENABLE (1<<16)
>> +#define CNL_AUX_ANAOVRD1_LDO_BYPASS (1<<23)
>
> I can't actually find these registers in bspec. How do you come up with
> the names and stuff?
>
> Based on the offset they look like PHY registers to me, so probably
> should be placed somewhere around the existing PHY registers.
>
>> +
>> /* Per-pipe DDI Function Control */
>> #define _TRANS_DDI_FUNC_CTL_A 0x60400
>> #define _TRANS_DDI_FUNC_CTL_B 0x61400
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index 8315499452dc..9bf200e4885d 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -388,6 +388,15 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
>> I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), val | HSW_PWR_WELL_CTL_REQ(id));
>> hsw_wait_for_power_well_enable(dev_priv, power_well);
>>
>> + /* WA Display #1178 */
>
> Pls stick to a consistent w/a comment style.
Are you suggesting to change to "/* Display Wa #1178 */"? It seems the
most common style in the
codebase, although others are used.
- "Wa Display #number" is used in my other pending patch that was
based on first version by Rodrigo and
has 1 occurence
- "Display WA #number" has 13 occurrences
- "Display Wa #number" has 1 occurrence
- "WaName" has several occurrences and is by far the most common,
although I don't think all Wa have names
like these
Should I send a fix to these as well?
Thanks
Lucas De Marchi
>
>> + if (IS_CANNONLAKE(dev_priv) &&
>> + (id == CNL_DISP_PW_AUX_B || id == CNL_DISP_PW_AUX_C ||
>> + id == CNL_DISP_PW_AUX_D)) {
>> + val = I915_READ(CNL_AUX_ANAOVRD1(id));
>> + val |= CNL_AUX_ANAOVRD1_ENABLE | CNL_AUX_ANAOVRD1_LDO_BYPASS;
>> + I915_WRITE(CNL_AUX_ANAOVRD1(id), val);
>> + }
>> +
>> if (wait_fuses)
>> gen9_wait_for_power_well_fuses(dev_priv, pg);
>>
>> --
>> 2.14.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
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] drm/i915/cnl: WA Display #1178 to fix some type C dongles
2017-11-27 23:14 ` Lucas De Marchi
@ 2017-11-28 14:36 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2017-11-28 14:36 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx, Arthur J Runyan, Lucas De Marchi, Rodrigo Vivi
On Mon, Nov 27, 2017 at 03:14:21PM -0800, Lucas De Marchi wrote:
> On Thu, Nov 23, 2017 at 7:21 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Nov 22, 2017 at 10:55:14AM -0800, Lucas De Marchi wrote:
> >> WA Display #1178 is meant to fix Aux channel voltage swing too low with
> >> some type C dongles. Although it is for type C, HW engineers reported
> >> that it can be applied to all external ports even if they are not going
> >> to type C.
> >>
> >> For CNL we apply the workaround every time Aux B, C and D are powering
> >> up since they will lose the configuration when powered down.
> >>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Arthur J Runyan <arthur.j.runyan@intel.com>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>
> >> Since this is a workaround I think it would be desirable not to be
> >> so intrusive. The simplest thing to do is to add the
> >> IS_CANNONLAKE() and workaround as done here.
> >>
> >> An alternative that may be more elegant (but also more intrusive) is to
> >> declare a new ops for CNL for AUX B/C/D. Let me know what you think.
> >>
> >> For the type-C dongles that I have here it worked both with and without
> >> this patch, so bear in mind I couldn't actually reproduce the problem.
> >>
> >> drivers/gpu/drm/i915/i915_reg.h | 11 +++++++++++
> >> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++++++++
> >> 2 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index 96c80fa0fcac..32064605f82d 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -8332,6 +8332,17 @@ enum skl_power_gate {
> >> #define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1)
> >> #define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg)))
> >>
> >> +#define _CNL_AUX_REG_IDX(pw) ((pw - 1) >> 4)
> >> +#define _CNL_AUX_ANAOVRD1_B 0x162250
> >> +#define _CNL_AUX_ANAOVRD1_C 0x162210
> >> +#define _CNL_AUX_ANAOVRD1_D 0x1622D0
> >> +#define CNL_AUX_ANAOVRD1(pw) _MMIO(_PICK(_CNL_AUX_REG_IDX(pw), \
> >> + _CNL_AUX_ANAOVRD1_B, \
> >> + _CNL_AUX_ANAOVRD1_C, \
> >> + _CNL_AUX_ANAOVRD1_D))
> >> +#define CNL_AUX_ANAOVRD1_ENABLE (1<<16)
> >> +#define CNL_AUX_ANAOVRD1_LDO_BYPASS (1<<23)
> >
> > I can't actually find these registers in bspec. How do you come up with
> > the names and stuff?
> >
> > Based on the offset they look like PHY registers to me, so probably
> > should be placed somewhere around the existing PHY registers.
> >
> >> +
> >> /* Per-pipe DDI Function Control */
> >> #define _TRANS_DDI_FUNC_CTL_A 0x60400
> >> #define _TRANS_DDI_FUNC_CTL_B 0x61400
> >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> index 8315499452dc..9bf200e4885d 100644
> >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> @@ -388,6 +388,15 @@ static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
> >> I915_WRITE(HSW_PWR_WELL_CTL_DRIVER(id), val | HSW_PWR_WELL_CTL_REQ(id));
> >> hsw_wait_for_power_well_enable(dev_priv, power_well);
> >>
> >> + /* WA Display #1178 */
> >
> > Pls stick to a consistent w/a comment style.
>
> Are you suggesting to change to "/* Display Wa #1178 */"? It seems the
> most common style in the
> codebase, although others are used.
>
> - "Wa Display #number" is used in my other pending patch that was
> based on first version by Rodrigo and
> has 1 occurence
> - "Display WA #number" has 13 occurrences
I guess we should go for this one then. Also we should add the
:platform tags to these as well.
> - "Display Wa #number" has 1 occurrence
> - "WaName" has several occurrences and is by far the most common,
> although I don't think all Wa have names
> like these
These are generally the ones which have an entry in the w/a db. I think
we should keep using them as well, when they exist.
>
> Should I send a fix to these as well?
Please do.
--
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] 6+ messages in thread
end of thread, other threads:[~2017-11-28 14:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-22 18:55 [PATCH] drm/i915/cnl: WA Display #1178 to fix some type C dongles Lucas De Marchi
2017-11-22 19:45 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-11-22 21:39 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-11-23 15:21 ` [PATCH] " Ville Syrjälä
2017-11-27 23:14 ` Lucas De Marchi
2017-11-28 14:36 ` 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.