[Intel-gfx] [PATCH] [VPG HSW-A] drm/i915: BUN vol4g[DevHSW]
Daniel Vetter
daniel at ffwll.ch
Tue Aug 27 11:06:21 CEST 2013
On Tue, Aug 27, 2013 at 05:27:21AM +0000, Biswas, KoushikX wrote:
> Hi Paulo,
> There are some changes in voltage swing levels for Haswell platform. In
> current implementation, macro name are given after the swing values. But
> the values are changed for Haswell. This comment is added to avoid the
> confusion during signal level debug or other similar kind of activity.
If the swing/preemph meanings of the values changed, then I think we
should updated the #defines. If needed we need to add a new table, which
with our current code structure should be fairly ok to do.
Generally comments should only explain high-level stuff or tricky stuff,
but not the details of the code for two reaons: 1. code should be
self-explaining. 2. comments don't get run and so bitrot way too quickly
compared to code.
-Daniel
>
>
> Thanks and regards,
>
> Koushik
>
> -----Original Message-----
> From: Paulo Zanoni [mailto:przanoni at gmail.com]
> Sent: Tuesday, August 27, 2013 12:46 AM
> To: Biswas, KoushikX
> Cc: Intel Graphics Development
> Subject: Re: [Intel-gfx] [PATCH] [VPG HSW-A] drm/i915: BUN vol4g[DevHSW]
>
> 2013/8/26 Koushik Biswas <koushikx.biswas at intel.com>:
> > From: koushik <koushikx.biswas at intel.com>
> >
> > WW43 2012 - DDI buffer translation corrections
> > WW36 2012 - Added HDMI voltage swing (not implemented
> > for HDMI)
> >
> > Added comments with voltage swing, pre-emphasis, transition and
> > non-transition values in form of table for reference. This values are
> > applicable only for HSW platform.
>
>
> But why exactly do we need this comment? We already have the
> intel_hsw_signal_levels() function (inside intel_dp.c) which is basically the implementation of your comment.
>
>
> >
> > Signed-off-by: koushik <koushikx.biswas at intel.com>
> > Change-Id: I0cff220c7d047f41b2a96b3e3880b4029550d458
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 31 +++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_dp.c | 32 ++++++++++++++++++++++++++++++++
> > 2 files changed, 63 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 5131517..0de236e 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -128,6 +128,37 @@ void intel_prepare_ddi(struct drm_device *dev)
> > intel_prepare_ddi_buffers(dev, PORT_E, true); }
> >
> > +
> > +/* Updating the new table in comments as it doesn't cause any logic
> > +change */
> > +
> > +/************* For HSW Voltage swing levels
> > +***************************/
> > +/********************************************************************
> > +**/
> > +/*___________________________________________________________________
> > +_*/ /*|Entry|Voltage|Pre-emphasis|Non-Transition|Transition
> > +|Pre-emphasis|*/
> > +/*| |Swing |level |mV diff p-p |mV diff p-p|db |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|0 |0 |0 |400 |400 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|1 |0 |1 |400 |600 |3.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|2 |0 |2 |400 |800 |6 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|3 |0 |3 |400 |1000 |8 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|4 |1 |0 |600 |600 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|5 |1 |1 |600 |900 |3.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|6 |1 |2 |600 |1000 |4.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|7 |2 |0 |800 |800 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|8 |2 |1 |1000 |1000 |2 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|9 | Entry 9 is only used for HDMI and DVI |*/
> > +/*|------------------------------------------------------------------
> > +|*/
> > +/********************************************************************
> > +**/
> > +
> > static const long hsw_ddi_buf_ctl_values[] = {
> > DDI_BUF_EMP_400MV_0DB_HSW,
> > DDI_BUF_EMP_400MV_3_5DB_HSW,
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 9fd7f90..fa73fb1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1809,6 +1809,38 @@ intel_gen7_edp_signal_levels(uint8_t train_set)
> > }
> > }
> >
> > +
> > +/* Updating the new table in comments as it doesn't cause any logic
> > +change */
> > +
> > +/************* For HSW Voltage swing levels
> > +***************************/
> > +/********************************************************************
> > +**/
> > +/*___________________________________________________________________
> > +_*/ /*|Entry|Voltage|Pre-emphasis|Non-Transition|Transition
> > +|Pre-emphasis|*/
> > +/*| |Swing |level |mV diff p-p |mV diff p-p|db |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|0 |0 |0 |400 |400 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|1 |0 |1 |400 |600 |3.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|2 |0 |2 |400 |800 |6 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|3 |0 |3 |400 |1000 |8 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|4 |1 |0 |600 |600 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|5 |1 |1 |600 |900 |3.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|6 |1 |2 |600 |1000 |4.5 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|7 |2 |0 |800 |800 |0 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|8 |2 |1 |1000 |1000 |2 |*/
> > +/*|------------------------------------------------------------------|*/
> > +/*|9 | Entry 9 is only used for HDMI and DVI |*/
> > +/*|------------------------------------------------------------------
> > +|*/
> > +/********************************************************************
> > +**/
> > +
> > +
> > /* Gen7.5's (HSW) DP voltage swing and pre-emphasis control */
> > static uint32_t intel_hsw_signal_levels(uint8_t train_set)
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list