public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 02/14] drm/i915: Clean up M/N register defines
Date: Thu, 27 Jan 2022 13:41:36 +0200	[thread overview]
Message-ID: <87lez1wglr.fsf@intel.com> (raw)
In-Reply-To: <YfKC1RR62rWp/M3G@intel.com>

On Thu, 27 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Jan 27, 2022 at 01:17:21PM +0200, Jani Nikula wrote:
>> On Thu, 27 Jan 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Use REG_GENMASK() & co. for the M/N register values. There are
>> > also a lot of weird unused defines (eg. *_OFFSET) we can just
>> > throw out.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++-----
>> >  drivers/gpu/drm/i915/i915_reg.h              | 22 +++-----------------
>> >  2 files changed, 8 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> > index f76faa195cb9..d91164d1eb92 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> > @@ -3865,11 +3865,11 @@ static void intel_get_m_n(struct drm_i915_private *i915,
>> >  			  i915_reg_t data_m_reg, i915_reg_t data_n_reg,
>> >  			  i915_reg_t link_m_reg, i915_reg_t link_n_reg)
>> >  {
>> > -	m_n->link_m = intel_de_read(i915, link_m_reg);
>> > -	m_n->link_n = intel_de_read(i915, link_n_reg);
>> > -	m_n->gmch_m = intel_de_read(i915, data_m_reg) & ~TU_SIZE_MASK;
>> > -	m_n->gmch_n = intel_de_read(i915, data_n_reg);
>> > -	m_n->tu = ((intel_de_read(i915, data_m_reg) & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
>> > +	m_n->link_m = intel_de_read(i915, link_m_reg) & DATA_LINK_M_N_MASK;
>> > +	m_n->link_n = intel_de_read(i915, link_n_reg) & DATA_LINK_M_N_MASK;
>> > +	m_n->gmch_m = intel_de_read(i915, data_m_reg) & DATA_LINK_M_N_MASK;
>> > +	m_n->gmch_n = intel_de_read(i915, data_n_reg) & DATA_LINK_M_N_MASK;
>> > +	m_n->tu = REG_FIELD_GET(TU_SIZE_MASK, intel_de_read(i915, data_m_reg)) + 1;
>> 
>> The commit message might mention we throw some bits away while reading.
>
> Right, forgot to note that.
>
>> 
>> A follow-up could perhasps axe the double read of the data_m_reg, but
>> *shrug*.
>
> I was going back and forth between keeping the double read vs. reading
> each just once here vs. reading straight in the callers and just passing
> the values through (would avoid needing i915_reg_t for the prototype
> in the headrer in a later patch). Somehow none of them tasted quit right
> so opted to keep the current thing for now.

Yeah, not a big deal. Also now that i915_reg_t is in i915_reg_defs.h I
think the overhead for that is neglible.

BR,
Jani.

>
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> 
>> >  }
>> >  
>> >  static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 2e4dd9db63fe..ec48406eb37a 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -5209,16 +5209,14 @@ enum {
>> >  #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 */
>> > -#define  TU_SIZE_SHIFT		25
>> > -#define  TU_SIZE_MASK           (0x3f << 25)
>> > +#define  TU_SIZE_MASK		REG_GENMASK(30, 25)
>> > +#define  TU_SIZE(x)		REG_FIELD_PREP(TU_SIZE_MASK, (x) - 1) /* default size 64 */
>> >  
>> > -#define  DATA_LINK_M_N_MASK	(0xffffff)
>> > +#define  DATA_LINK_M_N_MASK	REG_GENMASK(23, 0)
>> >  #define  DATA_LINK_N_MAX	(0x800000)
>> >  
>> >  #define _PIPEA_DATA_N_G4X	0x70054
>> >  #define _PIPEB_DATA_N_G4X	0x71054
>> > -#define   PIPE_GMCH_DATA_N_MASK			(0xffffff)
>> >  
>> >  /*
>> >   * Computing Link M and N values for the Display Port link
>> > @@ -5233,11 +5231,8 @@ enum {
>> >  
>> >  #define _PIPEA_LINK_M_G4X	0x70060
>> >  #define _PIPEB_LINK_M_G4X	0x71060
>> > -#define   PIPEA_DP_LINK_M_MASK			(0xffffff)
>> > -
>> >  #define _PIPEA_LINK_N_G4X	0x70064
>> >  #define _PIPEB_LINK_N_G4X	0x71064
>> > -#define   PIPEA_DP_LINK_N_MASK			(0xffffff)
>> >  
>> >  #define PIPE_DATA_M_G4X(pipe) _MMIO_PIPE(pipe, _PIPEA_DATA_M_G4X, _PIPEB_DATA_M_G4X)
>> >  #define PIPE_DATA_N_G4X(pipe) _MMIO_PIPE(pipe, _PIPEA_DATA_N_G4X, _PIPEB_DATA_N_G4X)
>> > @@ -6840,24 +6835,13 @@ enum {
>> >  
>> >  
>> >  #define _PIPEA_DATA_M1		0x60030
>> > -#define  PIPE_DATA_M1_OFFSET    0
>> >  #define _PIPEA_DATA_N1		0x60034
>> > -#define  PIPE_DATA_N1_OFFSET    0
>> > -
>> >  #define _PIPEA_DATA_M2		0x60038
>> > -#define  PIPE_DATA_M2_OFFSET    0
>> >  #define _PIPEA_DATA_N2		0x6003c
>> > -#define  PIPE_DATA_N2_OFFSET    0
>> > -
>> >  #define _PIPEA_LINK_M1		0x60040
>> > -#define  PIPE_LINK_M1_OFFSET    0
>> >  #define _PIPEA_LINK_N1		0x60044
>> > -#define  PIPE_LINK_N1_OFFSET    0
>> > -
>> >  #define _PIPEA_LINK_M2		0x60048
>> > -#define  PIPE_LINK_M2_OFFSET    0
>> >  #define _PIPEA_LINK_N2		0x6004c
>> > -#define  PIPE_LINK_N2_OFFSET    0
>> >  
>> >  /* PIPEB timing regs are same start from 0x61000 */
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2022-01-27 11:41 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-27  9:32 [Intel-gfx] [PATCH 00/14] drm/i915: M/N cleanup Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 01/14] drm/i915: Extract intel_{get,set}_m_n() Ville Syrjala
2022-01-27 11:11   ` [Intel-gfx] [PATCH 01/14] drm/i915: Extract intel_{get, set}_m_n() Jani Nikula
2022-01-27  9:32 ` [Intel-gfx] [PATCH 02/14] drm/i915: Clean up M/N register defines Ville Syrjala
2022-01-27 11:17   ` Jani Nikula
2022-01-27 11:32     ` Ville Syrjälä
2022-01-27 11:41       ` Jani Nikula [this message]
2022-01-27 12:02   ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2022-01-27 17:36   ` [Intel-gfx] [PATCH " kernel test robot
2022-01-27  9:32 ` [Intel-gfx] [PATCH 03/14] drm/i915: s/gmch_{m,n}/data_{m,n}/ Ville Syrjala
2022-01-27 11:18   ` Jani Nikula
2022-01-27  9:32 ` [Intel-gfx] [PATCH 04/14] drm/i915: Move drrs hardware bit frobbing to small helpers Ville Syrjala
2022-01-27 11:23   ` Jani Nikula
2022-01-27 11:24     ` Jani Nikula
2022-01-27 11:35       ` Ville Syrjälä
2022-01-27 11:42         ` Jani Nikula
2022-01-27  9:32 ` [Intel-gfx] [PATCH 05/14] drm/i915: Make M/N set/get a bit more direct Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 06/14] drm/i915: Move PCH transcoder M/N setup into the PCH code Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 07/14] drm/i915: Move M/N setup to a more logical place on ddi platforms Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 08/14] drm/i915: Extract {i9xx, ilk}_configure_cpu_transcoder() Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 09/14] drm/i915: Add fdi_m2_n2 Ville Syrjala
2022-01-27  9:32 ` [Intel-gfx] [PATCH 10/14] drm/i915: Program FDI RX TUSIZE2 Ville Syrjala
2022-01-27  9:33 ` [Intel-gfx] [PATCH 11/14] drm/i915: Dump dp_m2_n2 always Ville Syrjala
2022-01-27  9:33 ` [Intel-gfx] [PATCH 12/14] drm/i915: Extract can_enable_drrs() Ville Syrjala
2022-01-27  9:33 ` [Intel-gfx] [PATCH 13/14] drm/i915: Set DP M2/N2 equal to M1/N1 when not doing DRRS Ville Syrjala
2022-01-27  9:33 ` [Intel-gfx] [PATCH 14/14] drm/i915: Always check dp_m2_n2 on pre-bdw Ville Syrjala
2022-01-27 11:01 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: M/N cleanup Patchwork
2022-01-27 14:43 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: M/N cleanup (rev2) Patchwork
2022-01-27 15:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-01-27 19:55 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

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=87lez1wglr.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox