Message ID | 20170613193350.10528-8-paulo.r.zanoni@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote: > Do it just like we do with _PICK and _PICK3, so our code looks a > little more uniform. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index a97af4a..3fb7b63 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > } > > #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a))) > +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \ > + (a) + dev_priv->info.display_mmio_offset) > #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index]) > > #define _PIPE(pipe, a, b) _PICK(pipe, a, b) > #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) > -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \ > - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \ > - dev_priv->info.display_mmio_offset) > +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \ > + reg)) > #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__)) > > #define _PLANE(plane, a, b) _PICK(plane, a, b) > @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > > #define _TRANS(tran, a, b) _PICK(tran, a, b) > #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) > -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \ > - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \ > - dev_priv->info.display_mmio_offset) > +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \ > + dev_priv->info.trans_offsets, reg)) No love for cursor/palette offsets? > > #define _PORT(port, a, b) _PICK(port, a, b) > #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) > -- > 2.9.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em Qua, 2017-06-14 às 18:16 +0300, Ville Syrjälä escreveu: > On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote: > > Do it just like we do with _PICK and _PICK3, so our code looks a > > little more uniform. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index a97af4a..3fb7b63 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -49,13 +49,14 @@ static inline bool > > i915_mmio_reg_valid(i915_reg_t reg) > > } > > > > #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a))) > > +#define _PICK2(__index, __offsets, a) (__offsets[__index] - > > __offsets[0] + \ > > + (a) + dev_priv- > > >info.display_mmio_offset) > > #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ > > })[__index]) > > > > #define _PIPE(pipe, a, b) _PICK(pipe, a, b) > > #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) > > -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv- > > >info.pipe_offsets[pipe] - \ > > - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \ > > - dev_priv->info.display_mmio_offset) > > +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv- > > >info.pipe_offsets, \ > > + reg)) > > #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__)) > > > > #define _PLANE(plane, a, b) _PICK(plane, a, b) > > @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t > > reg) > > > > #define _TRANS(tran, a, b) _PICK(tran, a, b) > > #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) > > -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv- > > >info.trans_offsets[(pipe)] - \ > > - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \ > > - dev_priv->info.display_mmio_offset) > > +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \ > > + dev_priv- > > >info.trans_offsets, reg)) > > No love for cursor/palette offsets? I missed them. _CURSOR definitely fits the model. I'll see what we can do with PALLETTE. > > > > > #define _PORT(port, a, b) _PICK(port, a, b) > > #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) > > -- > > 2.9.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On Wed, Jun 14, 2017 at 8:16 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote: >> Do it just like we do with _PICK and _PICK3, so our code looks a >> little more uniform. >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index a97af4a..3fb7b63 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >> } >> yeap, as you predicted on the cover letter here starts the name bikeshedings ;) >> #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a))) for me this is more like a "guess" based on the gap, but specially I have concern about reusing the name that was for a totally different use. PICK before this list is a "select from a list" and after this series is "select base on uniform gap" >> +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \ >> + (a) + dev_priv->info.display_mmio_offset) and this is a select based on common base offset. >> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index]) and this is actual pick from a list also I don't like the numbers as versions of the macro... I'd prefer to use just in number of arguments as before. so, what about: s/PICK(/PICK_GAP s/PICK2/PICK_OFFSET s/PICK3/PICK_LIST ? or s/PICK(/SELECT_GAP s/PICK2/SELECT_OFFSET s/PICK3/SELECT_LIST ? >> >> #define _PIPE(pipe, a, b) _PICK(pipe, a, b) >> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) >> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \ >> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \ >> - dev_priv->info.display_mmio_offset) >> +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \ >> + reg)) >> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__)) >> >> #define _PLANE(plane, a, b) _PICK(plane, a, b) >> @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >> >> #define _TRANS(tran, a, b) _PICK(tran, a, b) >> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) >> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \ >> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \ >> - dev_priv->info.display_mmio_offset) >> +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \ >> + dev_priv->info.trans_offsets, reg)) > > No love for cursor/palette offsets? > >> >> #define _PORT(port, a, b) _PICK(port, a, b) >> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) >> -- >> 2.9.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 14 Jun 2017, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Wed, Jun 14, 2017 at 8:16 AM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: >> On Tue, Jun 13, 2017 at 04:33:50PM -0300, Paulo Zanoni wrote: >>> Do it just like we do with _PICK and _PICK3, so our code looks a >>> little more uniform. >>> >>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >>> index a97af4a..3fb7b63 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >>> } >>> > > yeap, as you predicted on the cover letter here starts the name bikeshedings ;) > >>> #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a))) > > for me this is more like a "guess" based on the gap, but specially > I have concern about reusing the name that was for a totally different use. > PICK before this list is a "select from a list" and after this series is > "select base on uniform gap" > >>> +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \ >>> + (a) + dev_priv->info.display_mmio_offset) > > and this is a select based on common base offset. > >>> #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index]) > > and this is actual pick from a list > > also I don't like the numbers as versions of the macro... > I'd prefer to use just in number of arguments as before. > > so, what about: > > s/PICK(/PICK_GAP > s/PICK2/PICK_OFFSET > s/PICK3/PICK_LIST Just _LINEAR, _ARRAY, _PICK. Or PICK_LINEAR, PICK_ARRAY, PICK_VA (or PICK_ARGS). BR, Jani. > > ? > > or > > s/PICK(/SELECT_GAP > s/PICK2/SELECT_OFFSET > s/PICK3/SELECT_LIST > > ? > >>> >>> #define _PIPE(pipe, a, b) _PICK(pipe, a, b) >>> #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) >>> -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \ >>> - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \ >>> - dev_priv->info.display_mmio_offset) >>> +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \ >>> + reg)) >>> #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__)) >>> >>> #define _PLANE(plane, a, b) _PICK(plane, a, b) >>> @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >>> >>> #define _TRANS(tran, a, b) _PICK(tran, a, b) >>> #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) >>> -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \ >>> - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \ >>> - dev_priv->info.display_mmio_offset) >>> +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \ >>> + dev_priv->info.trans_offsets, reg)) >> >> No love for cursor/palette offsets? >> >>> >>> #define _PORT(port, a, b) _PICK(port, a, b) >>> #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b)) >>> -- >>> 2.9.4 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> >> -- >> Ville Syrjälä >> Intel OTC >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a97af4a..3fb7b63 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -49,13 +49,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) } #define _PICK(__index, a, b) ((a) + (__index) * ((b) - (a))) +#define _PICK2(__index, __offsets, a) (__offsets[__index] - __offsets[0] + \ + (a) + dev_priv->info.display_mmio_offset) #define _PICK3(__index, ...) (((const u32 []){ __VA_ARGS__ })[__index]) #define _PIPE(pipe, a, b) _PICK(pipe, a, b) #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b)) -#define _MMIO_PIPE2(pipe, reg) _MMIO(dev_priv->info.pipe_offsets[pipe] - \ - dev_priv->info.pipe_offsets[PIPE_A] + (reg) + \ - dev_priv->info.display_mmio_offset) +#define _MMIO_PIPE2(pipe, reg) _MMIO(_PICK2(pipe, dev_priv->info.pipe_offsets, \ + reg)) #define _MMIO_PIPE3(pipe, ...) _MMIO(_PICK3(pipe, __VA_ARGS__)) #define _PLANE(plane, a, b) _PICK(plane, a, b) @@ -63,9 +64,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define _TRANS(tran, a, b) _PICK(tran, a, b) #define _MMIO_TRANS(tran, a, b) _MMIO(_TRANS(tran, a, b)) -#define _MMIO_TRANS2(pipe, reg) _MMIO(dev_priv->info.trans_offsets[(pipe)] - \ - dev_priv->info.trans_offsets[TRANSCODER_A] + (reg) + \ - dev_priv->info.display_mmio_offset) +#define _MMIO_TRANS2(tran, reg) _MMIO(_PICK2(tran, \ + dev_priv->info.trans_offsets, reg)) #define _PORT(port, a, b) _PICK(port, a, b) #define _MMIO_PORT(port, a, b) _MMIO(_PORT(port, a, b))
Do it just like we do with _PICK and _PICK3, so our code looks a little more uniform. Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)