Re: [PATCH 1/7] drm: Renaming DP training vswing pre emph defines

[Damien Lespiau <damien.lespiau@intel.com>]

On Tue, Aug 26, 2014 at 01:28:19PM +0200, Thierry Reding wrote:
> On Fri, Aug 08, 2014 at 04:23:40PM +0530, sonika.jindal@intel.com wrote:
> > From: Sonika Jindal <sonika.jindal@intel.com>
> > 
> > Adding new defines, older one will be removed in the last patch in the series.
> > This is to rename the defines to have levels instead of values for vswing and
> > pre-emph levels as the values may differ in other scenarios like low vswing of
> > eDP1.4 where the values are different.
> > 
> > Done using following cocci patch for each define:
> > @@
> > @@
> > 
> >  # define DP_TRAIN_VOLTAGE_SWING_400     (0 << 0)
> > + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0     (0 << 0)
> 
> Could this perhaps be simply:
> 
> 	#define DP_TRAIN_VOLTAGE_SWING(x) ((x) << 0)
> 
> As it is, there's no information about the value within the symbolic
> name anyway, so _LEVEL_* really isn't that useful and keeping several
> macros for each value seems isn't either.

The _LEVEL_ part is quite important IMHO, that's what changes between those
different defines, controlling a level shifter, somewhere.

So we're left with

  #define DP_TRAIN_VOLTAGE_SWING_LEVEL_0     (0 << 0)

Vs

  #define DP_TRAIN_VOLTAGE_SWING_LEVEL(x)     ((x) << 0)

The second variant doesn't really bring much more clarity? Can we just
go with the first?

> An alternative would be to provide a second set of defines for eDP 1.4
> where the name implies the meaning and then use them as appropriate.

We went through the idea as well and:

I actually think the nominal voltage swing and pre-emph values are quite
misleading. The hw is free to implement a wildly different set of voltage
swing/pre-emph values.

eDP 1.4 changes those nominal values as described in the cover letter,
but there again, the actual hw implementation can choose fairly
different values than the nominal ones.

Also, the DP 1.2 spec documents this field as (see address 103h):

TRAINING_LANE0_SET : Link Training Control_Lane0
  Bits 1:0 = VOLTAGE SWING SET
    00 –Voltage swing level 0
    01 –Voltage swing level 1
    10 –Voltage swing level 2
    11 –Voltage swing level 3

So, in that sense, we're closer to the latest spec with those LEVEL_X
defines.

HTH,

-- 
Damien
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel