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@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> >> Cc: Arthur J Runyan <arthur.j.runyan@xxxxxxxxx>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >> ---
> >>
> >> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
