All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915/reg: split out vblank/scanline regs
Date: Wed, 14 Dec 2022 15:42:29 +0200	[thread overview]
Message-ID: <Y5nSxSencoj8515T@intel.com> (raw)
In-Reply-To: <71aa10a6274199165e0b218ae6331ce3739aceed.1670855299.git.jani.nikula@intel.com>

On Mon, Dec 12, 2022 at 04:29:25PM +0200, Jani Nikula wrote:
> Reduce clutter in i915_reg.h by splitting out the vblank/scanline
> registers.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crt.c      |  1 +
>  drivers/gpu/drm/i915/display/intel_vblank.c   |  1 +
>  .../gpu/drm/i915/display/intel_vblank_regs.h  | 42 +++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/cmd_parser.c         |  1 +
>  drivers/gpu/drm/i915/gvt/display.c            |  1 +
>  drivers/gpu/drm/i915/gvt/handlers.c           |  1 +
>  drivers/gpu/drm/i915/i915_reg.h               | 35 ----------------
>  drivers/gpu/drm/i915/intel_gvt_mmio_table.c   |  1 +
>  8 files changed, 48 insertions(+), 35 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vblank_regs.h
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 7267ffc7f539..e7482fc9f726 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -49,6 +49,7 @@
>  #include "intel_hotplug.h"
>  #include "intel_pch_display.h"
>  #include "intel_pch_refclk.h"
> +#include "intel_vblank_regs.h"

That doesn't match any actual hardware block structure
so not really a fan. We could perhaps try something
like intel_{pipe,transcoder}_regs.h, but IIRC some of the
registers have more or less been moved between the pipe
vs. transcoder so not sure that makes total sense either.
Perhaps just intel_de_regs.h/etc. could be sufficient for
most things on this level?

>  
>  /* Here's the desired hotplug mode */
>  #define ADPA_HOTPLUG_BITS (ADPA_CRT_HOTPLUG_PERIOD_128 |		\
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 729c39180469..b017c2d00a3b 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -8,6 +8,7 @@
>  #include "intel_de.h"
>  #include "intel_display_types.h"
>  #include "intel_vblank.h"
> +#include "intel_vblank_regs.h"
>  
>  /*
>   * This timing diagram depicts the video signal in and
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank_regs.h b/drivers/gpu/drm/i915/display/intel_vblank_regs.h
> new file mode 100644
> index 000000000000..a851255ac2f1
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_vblank_regs.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __INTEL_VBLANK_REGS_H__
> +#define __INTEL_VBLANK_REGS_H__
> +
> +#include "intel_display_reg_defs.h"
> +
> +#define _PIPEADSL		0x70000
> +#define   PIPEDSL_CURR_FIELD	REG_BIT(31) /* ctg+ */
> +#define   PIPEDSL_LINE_MASK	REG_GENMASK(19, 0)
> +#define PIPEDSL(pipe)		_MMIO_PIPE2(pipe, _PIPEADSL)
> +
> +/*
> + * The two pipe frame counter registers are not synchronized, so reading a
> + * stable value is somewhat tricky. Use:
> + *
> + * u64 frame = intel_de_read64_2x32(dev_priv, PIPEFRAMEPIXEL(pipe), PIPEFRAME(pipe));
> + */
> +
> +#define _PIPEAFRAMEHIGH          0x70040
> +#define PIPEFRAME(pipe)		_MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH)
> +#define   PIPE_FRAME_HIGH_MASK	0x0000ffff
> +#define   PIPE_FRAME_HIGH_SHIFT	0
> +
> +#define _PIPEAFRAMEPIXEL	0x70044
> +#define PIPEFRAMEPIXEL(pipe)	_MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL)
> +#define   PIPE_FRAME_LOW_MASK	0xff000000
> +#define   PIPE_FRAME_LOW_SHIFT	24
> +#define   PIPE_PIXEL_MASK	0x00ffffff
> +#define   PIPE_PIXEL_SHIFT	0
> +
> +/* GM45+ just has to be different */
> +#define _PIPEA_FRMCOUNT_G4X	0x70040
> +#define PIPE_FRMCOUNT_G4X(pipe) _MMIO_PIPE2(pipe, _PIPEA_FRMCOUNT_G4X)
> +
> +#define _PIPEA_FLIPCOUNT_G4X	0x70044
> +#define PIPE_FLIPCOUNT_G4X(pipe) _MMIO_PIPE2(pipe, _PIPEA_FLIPCOUNT_G4X)
> +
> +#endif /* __INTEL_VBLANK_REGS_H__ */
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 0ebf5fbf0e39..8a72c75943c6 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -38,6 +38,7 @@
>  
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "display/intel_vblank_regs.h"
>  #include "gt/intel_engine_regs.h"
>  #include "gt/intel_gpu_commands.h"
>  #include "gt/intel_gt_regs.h"
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index c033249e73f4..894c8497961f 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -37,6 +37,7 @@
>  #include "gvt.h"
>  
>  #include "display/intel_dpio_phy.h"
> +#include "display/intel_vblank_regs.h"
>  
>  static int get_edp_pipe(struct intel_vgpu *vgpu)
>  {
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index 735fc83e7026..efd613c2e068 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -45,6 +45,7 @@
>  #include "display/intel_dmc_regs.h"
>  #include "display/intel_dpio_phy.h"
>  #include "display/intel_fbc.h"
> +#include "display/intel_vblank_regs.h"
>  #include "display/vlv_dsi_pll_regs.h"
>  #include "gt/intel_gt_regs.h"
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2b7a63754e4d..f3d9797ceb52 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3489,9 +3489,6 @@
>  /* Display & cursor control */
>  
>  /* Pipe A */
> -#define _PIPEADSL		0x70000
> -#define   PIPEDSL_CURR_FIELD	REG_BIT(31) /* ctg+ */
> -#define   PIPEDSL_LINE_MASK	REG_GENMASK(19, 0)
>  #define _PIPEACONF		0x70008
>  #define   PIPECONF_ENABLE			REG_BIT(31)
>  #define   PIPECONF_DOUBLE_WIDE			REG_BIT(30) /* pre-i965 */
> @@ -3616,9 +3613,6 @@
>  #define PIPE_DSI1_OFFSET	0x7b800
>  
>  #define PIPECONF(pipe)		_MMIO_PIPE2(pipe, _PIPEACONF)
> -#define PIPEDSL(pipe)		_MMIO_PIPE2(pipe, _PIPEADSL)
> -#define PIPEFRAME(pipe)		_MMIO_PIPE2(pipe, _PIPEAFRAMEHIGH)
> -#define PIPEFRAMEPIXEL(pipe)	_MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL)
>  #define PIPESTAT(pipe)		_MMIO_PIPE2(pipe, _PIPEASTAT)
>  
>  #define  _PIPEAGCMAX           0x70010
> @@ -4034,35 +4028,6 @@
>  #define  WM_LP_SPRITE_MASK	REG_GENMASK(10, 0)
>  #define  WM_LP_SPRITE(x)	REG_FIELD_PREP(WM_LP_SPRITE_MASK, (x))
>  
> -/*
> - * The two pipe frame counter registers are not synchronized, so
> - * reading a stable value is somewhat tricky. The following code
> - * should work:
> - *
> - *  do {
> - *    high1 = ((INREG(PIPEAFRAMEHIGH) & PIPE_FRAME_HIGH_MASK) >>
> - *             PIPE_FRAME_HIGH_SHIFT;
> - *    low1 =  ((INREG(PIPEAFRAMEPIXEL) & PIPE_FRAME_LOW_MASK) >>
> - *             PIPE_FRAME_LOW_SHIFT);
> - *    high2 = ((INREG(PIPEAFRAMEHIGH) & PIPE_FRAME_HIGH_MASK) >>
> - *             PIPE_FRAME_HIGH_SHIFT);
> - *  } while (high1 != high2);
> - *  frame = (high1 << 8) | low1;
> - */
> -#define _PIPEAFRAMEHIGH          0x70040
> -#define   PIPE_FRAME_HIGH_MASK    0x0000ffff
> -#define   PIPE_FRAME_HIGH_SHIFT   0
> -#define _PIPEAFRAMEPIXEL         0x70044
> -#define   PIPE_FRAME_LOW_MASK     0xff000000
> -#define   PIPE_FRAME_LOW_SHIFT    24
> -#define   PIPE_PIXEL_MASK         0x00ffffff
> -#define   PIPE_PIXEL_SHIFT        0
> -/* GM45+ just has to be different */
> -#define _PIPEA_FRMCOUNT_G4X	0x70040
> -#define _PIPEA_FLIPCOUNT_G4X	0x70044
> -#define PIPE_FRMCOUNT_G4X(pipe) _MMIO_PIPE2(pipe, _PIPEA_FRMCOUNT_G4X)
> -#define PIPE_FLIPCOUNT_G4X(pipe) _MMIO_PIPE2(pipe, _PIPEA_FLIPCOUNT_G4X)
> -
>  /* Cursor A & B regs */
>  #define _CURACNTR		0x70080
>  /* Old style CUR*CNTR flags (desktop 8xx) */
> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> index ce6b3c3b636a..604cc7ea8dac 100644
> --- a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> @@ -7,6 +7,7 @@
>  #include "display/intel_backlight_regs.h"
>  #include "display/intel_dmc_regs.h"
>  #include "display/intel_dpio_phy.h"
> +#include "display/intel_vblank_regs.h"
>  #include "display/vlv_dsi_pll_regs.h"
>  #include "gt/intel_gt_regs.h"
>  #include "gvt/gvt.h"
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

  parent reply	other threads:[~2022-12-14 13:42 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-12 14:29 [Intel-gfx] [PATCH 0/7] drm/i915: extract vblank/scanline code to a separate file Jani Nikula
2022-12-12 14:29 ` [Intel-gfx] [PATCH 1/7] drm/i915/irq: split out vblank/scanline code to intel_vblank.[ch] Jani Nikula
2022-12-12 14:29 ` [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline functions " Jani Nikula
2022-12-14  4:15   ` Murthy, Arun R
2022-12-14  9:15     ` Jani Nikula
2022-12-15  2:11       ` Murthy, Arun R
2022-12-15  9:12         ` Jani Nikula
2022-12-15  9:59           ` Murthy, Arun R
2022-12-15 10:20             ` Jani Nikula
2022-12-15 10:39               ` Murthy, Arun R
2022-12-15 11:03                 ` Jani Nikula
2022-12-12 14:29 ` [Intel-gfx] [PATCH 3/7] drm/i915/display: use common function for checking scanline is moving Jani Nikula
2022-12-14  4:01   ` Murthy, Arun R
2022-12-12 14:29 ` [Intel-gfx] [PATCH 4/7] drm/i915/hdmi: abstract scanline range wait into intel_vblank.[ch] Jani Nikula
2022-12-14  4:17   ` Murthy, Arun R
2022-12-14 14:16   ` Ville Syrjälä
2022-12-12 14:29 ` [Intel-gfx] [PATCH 5/7] drm/i915/vblank: use intel_de_read() Jani Nikula
2022-12-14  4:05   ` Murthy, Arun R
2022-12-12 14:29 ` [Intel-gfx] [PATCH 6/7] drm/i915/vblank: add and use intel_de_read64_2x32() to read vblank counter Jani Nikula
2022-12-12 14:29 ` [Intel-gfx] [PATCH 7/7] drm/i915/reg: split out vblank/scanline regs Jani Nikula
2022-12-14  4:24   ` Murthy, Arun R
2022-12-14 13:42   ` Ville Syrjälä [this message]
2022-12-12 14:46 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: extract vblank/scanline code to a separate file Patchwork
2022-12-12 14:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-12-12 15:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-12-14  4:12 ` [Intel-gfx] [PATCH 0/7] " Murthy, Arun R
2022-12-14  9:18   ` Jani Nikula
2022-12-15  2:05     ` Murthy, Arun R

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=Y5nSxSencoj8515T@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@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.