diff mbox

[7/7] drm/i915: extract a _PICK2 macro

Message ID 20170613193350.10528-8-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R June 13, 2017, 7:33 p.m. UTC 
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(-)

Comments

Ville Syrjälä June 14, 2017, 3:16 p.m. UTC | #1 
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
Zanoni, Paulo R June 14, 2017, 5:34 p.m. UTC | #2 
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
> 
>
Rodrigo Vivi June 14, 2017, 5:38 p.m. UTC | #3 
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
Jani Nikula June 15, 2017, 8:33 p.m. UTC | #4 
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 mbox

Patch

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))