On Mon, 07 Jan 2019, Aditya Swarup <aditya.swarup@xxxxxxxxx> wrote:
> On Mon, Dec 31, 2018 at 11:03:22AM +0200, Jani Nikula wrote:
>> On Fri, 14 Dec 2018, Aditya Swarup <aditya.swarup@xxxxxxxxx> wrote:
>> > CNL macros for register groups CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are
>> > configured incorrectly wrt definition of _CNL_PORT_TX_DW_GRP.
>>
>> What a mess.
>>
>> Please fix this by making CNL and ICL macros behave the same way. It's
>> silly to have the macro args reversed between them. I think dw first,
>> port second.
>>
>> BR,
>> Jani.
>
> I believe it should be the other way round for the macros to make sense.
> Port first, followed by dw and then ln. For example we can see in the following
> macro expansion:
> #define _ICL_PORT_CL_DW(dw, port) (_ICL_COMBOPHY(port) + \
> 4 * (dw))
>
> port is used before dw and we should follow that sequence.
It's not about the usage, it's about semantics. We group primarily by
function, secondarily by port/pipe/transcoder.
BR,
Jani.
>
> I have another patch ready which makes ICL macros consistent with CNL
> macros if you approve the sequence of (port, dw, ln).
>
> Let me know if you agree.
>
> Regards,
> Aditya Swarup
>>
>>
>> >
>> > Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
>> > Cc: Imre Deak <imre.deak@xxxxxxxxx>
>> > Signed-off-by: Aditya Swarup <aditya.swarup@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/i915_reg.h | 8 ++++----
>> > 1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> > index 0796526dc10f..db1332cd9dcd 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -1836,8 +1836,8 @@ enum i915_power_well_id {
>> > #define _ICL_PORT_TX_DW_LN(dw, ln, port) (_ICL_COMBOPHY(port) + \
>> > _ICL_PORT_TX_LN(ln) + 4 * (dw))
>> >
>> > -#define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(2, port))
>> > -#define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(2, port))
>> > +#define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(port, 2))
>> > +#define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(port, 2))
>> > #define ICL_PORT_TX_DW2_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(2, port))
>> > #define ICL_PORT_TX_DW2_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(2, port))
>> > #define ICL_PORT_TX_DW2_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(2, 0, port))
>> > @@ -1869,8 +1869,8 @@ enum i915_power_well_id {
>> > #define CURSOR_COEFF(x) ((x) << 0)
>> > #define CURSOR_COEFF_MASK (0x3F << 0)
>> >
>> > -#define CNL_PORT_TX_DW5_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(5, port))
>> > -#define CNL_PORT_TX_DW5_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(5, port))
>> > +#define CNL_PORT_TX_DW5_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(port, 5))
>> > +#define CNL_PORT_TX_DW5_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(port, 5))
>> > #define ICL_PORT_TX_DW5_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(5, port))
>> > #define ICL_PORT_TX_DW5_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(5, port))
>> > #define ICL_PORT_TX_DW5_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(5, 0, port))
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
