From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Add polish to VLV WM shift+mask operations
Date: Tue, 10 Mar 2015 14:48:42 -0700 [thread overview]
Message-ID: <54FF66BA.3090307@virtuousgeek.org> (raw)
In-Reply-To: <1425996988-5747-1-git-send-email-ville.syrjala@linux.intel.com>
On 03/10/2015 07:16 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Wrap the FW register value shift+mask operations into a macro to hide
> the ugliness a bit. Also might avoid bugs due to typos.
>
> Also rename all the primary/sprite plane low order bit masks to have the
> _VLV suffix, so that we can use the FW_WM_VLV() macro instead of the
> FW_WM() macro for them in a consistent manner. Cursor and all the high
> order bits are left to use the FW_WM() macro as there's no real
> confusion with them.
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 10 +++---
> drivers/gpu/drm/i915/intel_pm.c | 74 +++++++++++++++++++++++------------------
> 2 files changed, 46 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 495b22b..8ff039d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4161,25 +4161,25 @@ enum skl_disp_power_wells {
> #define DSPFW_SPRITED_WM1_SHIFT 24
> #define DSPFW_SPRITED_WM1_MASK (0xff<<24)
> #define DSPFW_SPRITED_SHIFT 16
> -#define DSPFW_SPRITED_MASK (0xff<<16)
> +#define DSPFW_SPRITED_MASK_VLV (0xff<<16)
> #define DSPFW_SPRITEC_WM1_SHIFT 8
> #define DSPFW_SPRITEC_WM1_MASK (0xff<<8)
> #define DSPFW_SPRITEC_SHIFT 0
> -#define DSPFW_SPRITEC_MASK (0xff<<0)
> +#define DSPFW_SPRITEC_MASK_VLV (0xff<<0)
> #define DSPFW8_CHV (VLV_DISPLAY_BASE + 0x700b8)
> #define DSPFW_SPRITEF_WM1_SHIFT 24
> #define DSPFW_SPRITEF_WM1_MASK (0xff<<24)
> #define DSPFW_SPRITEF_SHIFT 16
> -#define DSPFW_SPRITEF_MASK (0xff<<16)
> +#define DSPFW_SPRITEF_MASK_VLV (0xff<<16)
> #define DSPFW_SPRITEE_WM1_SHIFT 8
> #define DSPFW_SPRITEE_WM1_MASK (0xff<<8)
> #define DSPFW_SPRITEE_SHIFT 0
> -#define DSPFW_SPRITEE_MASK (0xff<<0)
> +#define DSPFW_SPRITEE_MASK_VLV (0xff<<0)
> #define DSPFW9_CHV (VLV_DISPLAY_BASE + 0x7007c) /* wtf #2? */
> #define DSPFW_PLANEC_WM1_SHIFT 24
> #define DSPFW_PLANEC_WM1_MASK (0xff<<24)
> #define DSPFW_PLANEC_SHIFT 16
> -#define DSPFW_PLANEC_MASK (0xff<<16)
> +#define DSPFW_PLANEC_MASK_VLV (0xff<<16)
> #define DSPFW_CURSORC_WM1_SHIFT 8
> #define DSPFW_CURSORC_WM1_MASK (0x3f<<16)
> #define DSPFW_CURSORC_SHIFT 0
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0e84558..8ac358d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -835,6 +835,11 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> display, cursor);
> }
>
> +#define FW_WM(value, plane) \
> + (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK)
> +#define FW_WM_VLV(value, plane) \
> + (((value) << DSPFW_ ## plane ## _SHIFT) & DSPFW_ ## plane ## _MASK_VLV)
> +
> static void vlv_write_wm_values(struct intel_crtc *crtc,
> const struct vlv_wm_values *wm)
> {
> @@ -848,50 +853,50 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> (wm->ddl[pipe].primary << DDL_PLANE_SHIFT));
>
> I915_WRITE(DSPFW1,
> - ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) |
> - ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & DSPFW_CURSORB_MASK) |
> - ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & DSPFW_PLANEB_MASK_VLV) |
> - ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & DSPFW_PLANEA_MASK_VLV));
> + FW_WM(wm->sr.plane, SR) |
> + FW_WM(wm->pipe[PIPE_B].cursor, CURSORB) |
> + FW_WM_VLV(wm->pipe[PIPE_B].primary, PLANEB) |
> + FW_WM_VLV(wm->pipe[PIPE_A].primary, PLANEA));
> I915_WRITE(DSPFW2,
> - ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & DSPFW_SPRITEB_MASK_VLV) |
> - ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & DSPFW_CURSORA_MASK) |
> - ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & DSPFW_SPRITEA_MASK_VLV));
> + FW_WM_VLV(wm->pipe[PIPE_A].sprite[1], SPRITEB) |
> + FW_WM(wm->pipe[PIPE_A].cursor, CURSORA) |
> + FW_WM_VLV(wm->pipe[PIPE_A].sprite[0], SPRITEA));
> I915_WRITE(DSPFW3,
> - ((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & DSPFW_CURSOR_SR_MASK));
> + FW_WM(wm->sr.cursor, CURSOR_SR));
>
> if (IS_CHERRYVIEW(dev_priv)) {
> I915_WRITE(DSPFW7_CHV,
> - ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
> - ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
> + FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
> + FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
> I915_WRITE(DSPFW8_CHV,
> - ((wm->pipe[PIPE_C].sprite[1] << DSPFW_SPRITEF_SHIFT) & DSPFW_SPRITEF_MASK) |
> - ((wm->pipe[PIPE_C].sprite[0] << DSPFW_SPRITEE_SHIFT) & DSPFW_SPRITEE_MASK));
> + FW_WM_VLV(wm->pipe[PIPE_C].sprite[1], SPRITEF) |
> + FW_WM_VLV(wm->pipe[PIPE_C].sprite[0], SPRITEE));
> I915_WRITE(DSPFW9_CHV,
> - ((wm->pipe[PIPE_C].primary << DSPFW_PLANEC_SHIFT) & DSPFW_PLANEC_MASK) |
> - ((wm->pipe[PIPE_C].cursor << DSPFW_CURSORC_SHIFT) & DSPFW_CURSORC_MASK));
> + FW_WM_VLV(wm->pipe[PIPE_C].primary, PLANEC) |
> + FW_WM(wm->pipe[PIPE_C].cursor, CURSORC));
> I915_WRITE(DSPHOWM,
> - (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
> - (((wm->pipe[PIPE_C].sprite[1] >> 8) << DSPFW_SPRITEF_HI_SHIFT) & DSPFW_SPRITEF_HI_MASK) |
> - (((wm->pipe[PIPE_C].sprite[0] >> 8) << DSPFW_SPRITEE_HI_SHIFT) & DSPFW_SPRITEE_HI_MASK) |
> - (((wm->pipe[PIPE_C].primary >> 8) << DSPFW_PLANEC_HI_SHIFT) & DSPFW_PLANEC_HI_MASK) |
> - (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
> - (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
> - (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
> - (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
> - (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
> - (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
> + FW_WM(wm->sr.plane >> 9, SR_HI) |
> + FW_WM(wm->pipe[PIPE_C].sprite[1] >> 8, SPRITEF_HI) |
> + FW_WM(wm->pipe[PIPE_C].sprite[0] >> 8, SPRITEE_HI) |
> + FW_WM(wm->pipe[PIPE_C].primary >> 8, PLANEC_HI) |
> + FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
> + FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
> + FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
> + FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
> + FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
> + FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
> } else {
> I915_WRITE(DSPFW7,
> - ((wm->pipe[PIPE_B].sprite[1] << DSPFW_SPRITED_SHIFT) & DSPFW_SPRITED_MASK) |
> - ((wm->pipe[PIPE_B].sprite[0] << DSPFW_SPRITEC_SHIFT) & DSPFW_SPRITEC_MASK));
> + FW_WM_VLV(wm->pipe[PIPE_B].sprite[1], SPRITED) |
> + FW_WM_VLV(wm->pipe[PIPE_B].sprite[0], SPRITEC));
> I915_WRITE(DSPHOWM,
> - (((wm->sr.plane >> 9) << DSPFW_SR_HI_SHIFT) & DSPFW_SR_HI_MASK) |
> - (((wm->pipe[PIPE_B].sprite[1] >> 8) << DSPFW_SPRITED_HI_SHIFT) & DSPFW_SPRITED_HI_MASK) |
> - (((wm->pipe[PIPE_B].sprite[0] >> 8) << DSPFW_SPRITEC_HI_SHIFT) & DSPFW_SPRITEC_HI_MASK) |
> - (((wm->pipe[PIPE_B].primary >> 8) << DSPFW_PLANEB_HI_SHIFT) & DSPFW_PLANEB_HI_MASK) |
> - (((wm->pipe[PIPE_A].sprite[1] >> 8) << DSPFW_SPRITEB_HI_SHIFT) & DSPFW_SPRITEB_HI_MASK) |
> - (((wm->pipe[PIPE_A].sprite[0] >> 8) << DSPFW_SPRITEA_HI_SHIFT) & DSPFW_SPRITEA_HI_MASK) |
> - (((wm->pipe[PIPE_A].primary >> 8) << DSPFW_PLANEA_HI_SHIFT) & DSPFW_PLANEA_HI_MASK));
> + FW_WM(wm->sr.plane >> 9, SR_HI) |
> + FW_WM(wm->pipe[PIPE_B].sprite[1] >> 8, SPRITED_HI) |
> + FW_WM(wm->pipe[PIPE_B].sprite[0] >> 8, SPRITEC_HI) |
> + FW_WM(wm->pipe[PIPE_B].primary >> 8, PLANEB_HI) |
> + FW_WM(wm->pipe[PIPE_A].sprite[1] >> 8, SPRITEB_HI) |
> + FW_WM(wm->pipe[PIPE_A].sprite[0] >> 8, SPRITEA_HI) |
> + FW_WM(wm->pipe[PIPE_A].primary >> 8, PLANEA_HI));
> }
>
> POSTING_READ(DSPFW1);
> @@ -899,6 +904,9 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
> dev_priv->wm.vlv = *wm;
> }
>
> +#undef FW_WM
> +#undef FW_WM_VLV
> +
> static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
> struct drm_plane *plane)
> {
>
I'd rather see the macros put in the header next to the FW definitions,
but that's not a blocker.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-03-10 21:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-10 14:16 [PATCH] drm/i915: Add polish to VLV WM shift+mask operations ville.syrjala
2015-03-10 15:02 ` [PATCH] drm/i915: Use FW_WM() macro for older gmch platforms too ville.syrjala
2015-03-10 21:50 ` Jesse Barnes
2015-03-11 10:05 ` Daniel Vetter
2015-03-10 21:48 ` Jesse Barnes [this message]
2015-03-11 15:02 ` [PATCH] drm/i915: Add polish to VLV WM shift+mask operations shuang.he
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54FF66BA.3090307@virtuousgeek.org \
--to=jbarnes@virtuousgeek.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.