[PATCH 4/5] drm/i915: Apply OCD to data/link m/n register #defines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

2013/5/1 Daniel Vetter <daniel.vetter at ffwll.ch>:
> - PCH_ prefix for pch registers on ibx/cpt/ppt.
> - Drop the DP_ from the link defines, redundant.
> - Drop the GMCH from the data defines and instead give the special g4x
>   registers a consistent _G4X postfix.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Yay, the new naming is way better. Thanks!

So since this is about OCD, I'll give my optional OCD bikesheds:
- Now the register addresses are not aligned on i915_reg.h anymore. I
suggest we align them again. We should also consider replacing all
those white spaces with tabs.
- We should have renamed the suspend/resume regfile registers too.
They're getting confusing now.
- We go over 80 columns in some new cases now. Do we have a policy on
when it's accepted on our driver?
- Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com> but I'd love to
see my bikesheds on v2 :)

I also noticed patches 3 and 4 don't apply anymore.

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 74 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/i915_ums.c      | 32 ++++++++--------
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++----
>  3 files changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2b2cb5f..44e1490 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2770,8 +2770,8 @@
>   * which is after the LUTs, so we want the bytes for our color format.
>   * For our current usage, this is always 3, one byte for R, G and B.
>   */
> -#define _PIPEA_GMCH_DATA_M                     0x70050
> -#define _PIPEB_GMCH_DATA_M                     0x71050
> +#define _PIPEA_DATA_M_G4X                      0x70050
> +#define _PIPEB_DATA_M_G4X                      0x71050
>
>  /* Transfer unit size for display port - 1, default is 0x3f (for TU size 64) */
>  #define  TU_SIZE(x)             (((x)-1) << 25) /* default size 64 */
> @@ -2781,8 +2781,8 @@
>  #define  DATA_LINK_M_N_MASK    (0xffffff)
>  #define  DATA_LINK_N_MAX       (0x800000)
>
> -#define _PIPEA_GMCH_DATA_N                     0x70054
> -#define _PIPEB_GMCH_DATA_N                     0x71054
> +#define _PIPEA_DATA_N_G4X                      0x70054
> +#define _PIPEB_DATA_N_G4X                      0x71054
>
>  /*
>   * Computing Link M and N values for the Display Port link
> @@ -2795,16 +2795,16 @@
>   * Attributes and VB-ID.
>   */
>
> -#define _PIPEA_DP_LINK_M                               0x70060
> -#define _PIPEB_DP_LINK_M                               0x71060
> +#define _PIPEA_LINK_M_G4X                              0x70060
> +#define _PIPEB_LINK_M_G4X                              0x71060
>
> -#define _PIPEA_DP_LINK_N                               0x70064
> -#define _PIPEB_DP_LINK_N                               0x71064
> +#define _PIPEA_LINK_N_G4X                              0x70064
> +#define _PIPEB_LINK_N_G4X                              0x71064
>
> -#define PIPE_GMCH_DATA_M(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_M, _PIPEB_GMCH_DATA_M)
> -#define PIPE_GMCH_DATA_N(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_N, _PIPEB_GMCH_DATA_N)
> -#define PIPE_DP_LINK_M(pipe) _PIPE(pipe, _PIPEA_DP_LINK_M, _PIPEB_DP_LINK_M)
> -#define PIPE_DP_LINK_N(pipe) _PIPE(pipe, _PIPEA_DP_LINK_N, _PIPEB_DP_LINK_N)
> +#define PIPE_DATA_M_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_M_G4X, _PIPEB_DATA_M_G4X)
> +#define PIPE_DATA_N_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_N_G4X, _PIPEB_DATA_N_G4X)
> +#define PIPE_LINK_M_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_M_G4X, _PIPEB_LINK_M_G4X)
> +#define PIPE_LINK_N_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_N_G4X, _PIPEB_LINK_N_G4X)
>
>  /* Display & cursor control */
>
> @@ -3947,14 +3947,14 @@
>  #define  TRANS_VSYNC_START_SHIFT 0
>  #define _PCH_TRANS_VSYNCSHIFT_A        0xe0028
>
> -#define _TRANSA_DATA_M1          0xe0030
> -#define _TRANSA_DATA_N1          0xe0034
> -#define _TRANSA_DATA_M2          0xe0038
> -#define _TRANSA_DATA_N2          0xe003c
> -#define _TRANSA_DP_LINK_M1       0xe0040
> -#define _TRANSA_DP_LINK_N1       0xe0044
> -#define _TRANSA_DP_LINK_M2       0xe0048
> -#define _TRANSA_DP_LINK_N2       0xe004c
> +#define _PCH_TRANSA_DATA_M1          0xe0030
> +#define _PCH_TRANSA_DATA_N1          0xe0034
> +#define _PCH_TRANSA_DATA_M2          0xe0038
> +#define _PCH_TRANSA_DATA_N2          0xe003c
> +#define _PCH_TRANSA_LINK_M1       0xe0040
> +#define _PCH_TRANSA_LINK_N1       0xe0044
> +#define _PCH_TRANSA_LINK_M2       0xe0048
> +#define _PCH_TRANSA_LINK_N2       0xe004c
>
>  /* Per-transcoder DIP controls */
>
> @@ -4040,23 +4040,23 @@
>  #define PCH_TRANS_VSYNCSHIFT(pipe) _PIPE(pipe, _PCH_TRANS_VSYNCSHIFT_A, \
>                                          _PCH_TRANS_VSYNCSHIFT_B)
>
> -#define _TRANSB_DATA_M1          0xe1030
> -#define _TRANSB_DATA_N1          0xe1034
> -#define _TRANSB_DATA_M2          0xe1038
> -#define _TRANSB_DATA_N2          0xe103c
> -#define _TRANSB_DP_LINK_M1       0xe1040
> -#define _TRANSB_DP_LINK_N1       0xe1044
> -#define _TRANSB_DP_LINK_M2       0xe1048
> -#define _TRANSB_DP_LINK_N2       0xe104c
> -
> -#define TRANSDATA_M1(pipe) _PIPE(pipe, _TRANSA_DATA_M1, _TRANSB_DATA_M1)
> -#define TRANSDATA_N1(pipe) _PIPE(pipe, _TRANSA_DATA_N1, _TRANSB_DATA_N1)
> -#define TRANSDATA_M2(pipe) _PIPE(pipe, _TRANSA_DATA_M2, _TRANSB_DATA_M2)
> -#define TRANSDATA_N2(pipe) _PIPE(pipe, _TRANSA_DATA_N2, _TRANSB_DATA_N2)
> -#define TRANSDPLINK_M1(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M1, _TRANSB_DP_LINK_M1)
> -#define TRANSDPLINK_N1(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N1, _TRANSB_DP_LINK_N1)
> -#define TRANSDPLINK_M2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_M2, _TRANSB_DP_LINK_M2)
> -#define TRANSDPLINK_N2(pipe) _PIPE(pipe, _TRANSA_DP_LINK_N2, _TRANSB_DP_LINK_N2)
> +#define _PCH_TRANSB_DATA_M1          0xe1030
> +#define _PCH_TRANSB_DATA_N1          0xe1034
> +#define _PCH_TRANSB_DATA_M2          0xe1038
> +#define _PCH_TRANSB_DATA_N2          0xe103c
> +#define _PCH_TRANSB_LINK_M1       0xe1040
> +#define _PCH_TRANSB_LINK_N1       0xe1044
> +#define _PCH_TRANSB_LINK_M2       0xe1048
> +#define _PCH_TRANSB_LINK_N2       0xe104c
> +
> +#define PCH_TRANS_DATA_M1(pipe) _PIPE(pipe, _PCH_TRANSA_DATA_M1, _PCH_TRANSB_DATA_M1)
> +#define PCH_TRANS_DATA_N1(pipe) _PIPE(pipe, _PCH_TRANSA_DATA_N1, _PCH_TRANSB_DATA_N1)
> +#define PCH_TRANS_DATA_M2(pipe) _PIPE(pipe, _PCH_TRANSA_DATA_M2, _PCH_TRANSB_DATA_M2)
> +#define PCH_TRANS_DATA_N2(pipe) _PIPE(pipe, _PCH_TRANSA_DATA_N2, _PCH_TRANSB_DATA_N2)
> +#define PCH_TRANS_LINK_M1(pipe) _PIPE(pipe, _PCH_TRANSA_LINK_M1, _PCH_TRANSB_LINK_M1)
> +#define PCH_TRANS_LINK_N1(pipe) _PIPE(pipe, _PCH_TRANSA_LINK_N1, _PCH_TRANSB_LINK_N1)
> +#define PCH_TRANS_LINK_M2(pipe) _PIPE(pipe, _PCH_TRANSA_LINK_M2, _PCH_TRANSB_LINK_M2)
> +#define PCH_TRANS_LINK_N2(pipe) _PIPE(pipe, _PCH_TRANSA_LINK_N2, _PCH_TRANSB_LINK_N2)
>
>  #define _PCH_TRANSACONF              0xf0008
>  #define _PCH_TRANSBCONF              0xf1008
> diff --git a/drivers/gpu/drm/i915/i915_ums.c b/drivers/gpu/drm/i915/i915_ums.c
> index 4168d2b..5ef30b2 100644
> --- a/drivers/gpu/drm/i915/i915_ums.c
> +++ b/drivers/gpu/drm/i915/i915_ums.c
> @@ -259,14 +259,14 @@ void i915_save_display_reg(struct drm_device *dev)
>                 dev_priv->regfile.saveDP_B = I915_READ(DP_B);
>                 dev_priv->regfile.saveDP_C = I915_READ(DP_C);
>                 dev_priv->regfile.saveDP_D = I915_READ(DP_D);
> -               dev_priv->regfile.savePIPEA_GMCH_DATA_M = I915_READ(_PIPEA_GMCH_DATA_M);
> -               dev_priv->regfile.savePIPEB_GMCH_DATA_M = I915_READ(_PIPEB_GMCH_DATA_M);
> -               dev_priv->regfile.savePIPEA_GMCH_DATA_N = I915_READ(_PIPEA_GMCH_DATA_N);
> -               dev_priv->regfile.savePIPEB_GMCH_DATA_N = I915_READ(_PIPEB_GMCH_DATA_N);
> -               dev_priv->regfile.savePIPEA_DP_LINK_M = I915_READ(_PIPEA_DP_LINK_M);
> -               dev_priv->regfile.savePIPEB_DP_LINK_M = I915_READ(_PIPEB_DP_LINK_M);
> -               dev_priv->regfile.savePIPEA_DP_LINK_N = I915_READ(_PIPEA_DP_LINK_N);
> -               dev_priv->regfile.savePIPEB_DP_LINK_N = I915_READ(_PIPEB_DP_LINK_N);
> +               dev_priv->regfile.savePIPEA_GMCH_DATA_M = I915_READ(_PIPEA_DATA_M_G4X);
> +               dev_priv->regfile.savePIPEB_GMCH_DATA_M = I915_READ(_PIPEB_DATA_M_G4X);
> +               dev_priv->regfile.savePIPEA_GMCH_DATA_N = I915_READ(_PIPEA_DATA_N_G4X);
> +               dev_priv->regfile.savePIPEB_GMCH_DATA_N = I915_READ(_PIPEB_DATA_N_G4X);
> +               dev_priv->regfile.savePIPEA_DP_LINK_M = I915_READ(_PIPEA_LINK_M_G4X);
> +               dev_priv->regfile.savePIPEB_DP_LINK_M = I915_READ(_PIPEB_LINK_M_G4X);
> +               dev_priv->regfile.savePIPEA_DP_LINK_N = I915_READ(_PIPEA_LINK_N_G4X);
> +               dev_priv->regfile.savePIPEB_DP_LINK_N = I915_READ(_PIPEB_LINK_N_G4X);
>         }
>         /* FIXME: regfile.save TV & SDVO state */
>
> @@ -282,14 +282,14 @@ void i915_restore_display_reg(struct drm_device *dev)
>
>         /* Display port ratios (must be done before clock is set) */
>         if (SUPPORTS_INTEGRATED_DP(dev)) {
> -               I915_WRITE(_PIPEA_GMCH_DATA_M, dev_priv->regfile.savePIPEA_GMCH_DATA_M);
> -               I915_WRITE(_PIPEB_GMCH_DATA_M, dev_priv->regfile.savePIPEB_GMCH_DATA_M);
> -               I915_WRITE(_PIPEA_GMCH_DATA_N, dev_priv->regfile.savePIPEA_GMCH_DATA_N);
> -               I915_WRITE(_PIPEB_GMCH_DATA_N, dev_priv->regfile.savePIPEB_GMCH_DATA_N);
> -               I915_WRITE(_PIPEA_DP_LINK_M, dev_priv->regfile.savePIPEA_DP_LINK_M);
> -               I915_WRITE(_PIPEB_DP_LINK_M, dev_priv->regfile.savePIPEB_DP_LINK_M);
> -               I915_WRITE(_PIPEA_DP_LINK_N, dev_priv->regfile.savePIPEA_DP_LINK_N);
> -               I915_WRITE(_PIPEB_DP_LINK_N, dev_priv->regfile.savePIPEB_DP_LINK_N);
> +               I915_WRITE(_PIPEA_DATA_M_G4X, dev_priv->regfile.savePIPEA_GMCH_DATA_M);
> +               I915_WRITE(_PIPEB_DATA_M_G4X, dev_priv->regfile.savePIPEB_GMCH_DATA_M);
> +               I915_WRITE(_PIPEA_DATA_N_G4X, dev_priv->regfile.savePIPEA_GMCH_DATA_N);
> +               I915_WRITE(_PIPEB_DATA_N_G4X, dev_priv->regfile.savePIPEB_GMCH_DATA_N);
> +               I915_WRITE(_PIPEA_LINK_M_G4X, dev_priv->regfile.savePIPEA_DP_LINK_M);
> +               I915_WRITE(_PIPEB_LINK_M_G4X, dev_priv->regfile.savePIPEB_DP_LINK_M);
> +               I915_WRITE(_PIPEA_LINK_N_G4X, dev_priv->regfile.savePIPEA_DP_LINK_N);
> +               I915_WRITE(_PIPEB_LINK_N_G4X, dev_priv->regfile.savePIPEB_DP_LINK_N);
>         }
>
>         /* Fences */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3edda85..df6c858 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4393,10 +4393,10 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int pipe = crtc->pipe;
>
> -       I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> -       I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
> -       I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
> -       I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
> +       I915_WRITE(PCH_TRANS_DATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> +       I915_WRITE(PCH_TRANS_DATA_N1(pipe), m_n->gmch_n);
> +       I915_WRITE(PCH_TRANS_LINK_M1(pipe), m_n->link_m);
> +       I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
>  }
>
>  static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> @@ -4413,10 +4413,10 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>                 I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>                 I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>         } else {
> -               I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> -               I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
> -               I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
> -               I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
> +               I915_WRITE(PIPE_DATA_M_G4X(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> +               I915_WRITE(PIPE_DATA_N_G4X(pipe), m_n->gmch_n);
> +               I915_WRITE(PIPE_LINK_M_G4X(pipe), m_n->link_m);
> +               I915_WRITE(PIPE_LINK_N_G4X(pipe), m_n->link_n);
>         }
>  }
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux