Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, Feb 16, 2022 at 09:36:02AM +, Hogander, Jouni wrote:
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
> From: Jouni Högander
>
> Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E
> port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps()
here:
// snip
else if (IS_DG2(dev_priv))
/*
* All four "combo" ports and the TC1 port (PHY E) use
* Synopsis PHYs.
*/
return phy <= PHY_E;
// snip
And this is actually the bug that we had. We wouldn't need to bring the
incomplete support for the 5th port if this single had changed: it's
often preferred to prepare the driver first and enable the port/phy as
the last step:
- return phy <= PHY_E;
+ return phy <= PHY_D;
With possibly a change in the commit above. Because in
drivers/gpu/drm/i915/display/intel_snps_phy.c we do:
intel_snps_phy_wait_for_calibration()
{
...
for_each_phy_masked(phy, ~0) {
if (!intel_phy_is_snps(i915, phy))
continue;
...
}
Relying on intel_phy_is_snps() to mask out the unavailable phys.
However, since now we almost have the extra port wired up, I'm not going
to push back on it. Let's just add a comment on the commit message.
And since going with this approach is also acked by Ville who preferred
to contain the additional mapping inside intel_phy_snps.c:
Reviewed-by: Lucas De Marchi
Lucas De Marchi
According to spec port E is "No connection". Better place to fix this
could be intel_phy_is_snps() itself?
> Fix this by handling PHY_E port seprately.
>
> Signed-off-by: Matt Roper
> Signed-off-by: Jouni Högander
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 6 --
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> index c60575cb5368..f08061c748b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct
> drm_i915_private *i915)
>if (!intel_phy_is_snps(i915, phy))
>continue;
>
> - if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
> + if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy),
>DG2_PHY_DP_TX_ACK_MASK,
> 25))
>drm_err(>drm, "SNPS PHY %c failed to
> calibrate after 25ms.\n",
>phy);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 4d12abb2d7ff..354c25f483cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9559,8 +9559,10 @@ enum skl_power_gate {
>
> #define _ICL_PHY_MISC_A 0x64C00
> #define _ICL_PHY_MISC_B 0x64C04
> -#define ICL_PHY_MISC(port)_MMIO_PORT(port, _ICL_PHY_MISC_A, \
> - _ICL_PHY_MISC_B)
> +#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if
> "PHY F" */
> +#define ICL_PHY_MISC(port)_MMIO_PORT(port, _ICL_PHY_MISC_A,
> _ICL_PHY_MISC_B)
> +#define DG2_PHY_MISC(port)((port) == PHY_E ?
> _MMIO(_DG2_PHY_MISC_TC1) : \
> + ICL_PHY_MISC(port))
> #define ICL_PHY_MISC_MUX_DDID(1 << 28)
> #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23)
> #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23,
> 20)
> --
> 2.20.1
BR,
Jouni Högander
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote: From: Jouni Högander Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port. Correct offset is 0x64C14. Fix this by handling PHY_E port seprately. order of the patch here is wrong. This patch should come before the patch initializing the 5th port. Then the commit message is not a fix. This can be done while applying since it's more an order to avoid breaking the tree. Lucas De Marchi
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, Feb 16, 2022 at 05:01:35PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 16, 2022 at 02:11:54PM +, Hogander, Jouni wrote:
> > On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote:
> > > On Wed, Feb 16, 2022 at 09:36:02AM +, Hogander, Jouni wrote:
> > > > On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
> > > > > On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
> > > > > > From: Jouni Högander
> > > > > >
> > > > > > Currently ICL_PHY_MISC macro is returning offset 0x64C10 for
> > > > > > PHY_E
> > > > > > port. Correct offset is 0x64C14.
> > > > >
> > > > > Why is it PHY_E and not PHY_F?
> > > >
> > > > This is a valid question. It seems we have followed
> > > > intel_phy_is_snps()
> > > > here:
> > > >
> > > > // snip
> > > > else if (IS_DG2(dev_priv))
> > > > /*
> > > > * All four "combo" ports and the TC1 port (PHY E) use
> > > > * Synopsis PHYs.
> > > > */
> > > > return phy <= PHY_E;
> > > > // snip
> > > >
> > > > According to spec port E is "No connection". Better place to fix
> > > > this
> > > > could be intel_phy_is_snps() itself?
> > >
> > > I think the crucial question is where are all the places that
> > > the results of intel_port_to_phy() get used.
> > >
> > > I do see that for all the actual snps phy registers we
> > > do want PHY_E, but maybe it would be better to have a local
> > > SNPS_PHY enum just for intel_snps_phy.c, and leave the other
> > > phy thing for everything else?
> > >
> > > Not sure if there is some other register we index with the
> > > phy that specifically wants PHY_E?
> >
> > I went through registers accesses in intel_snps_phy.c. It is actually
> > only this one register which offset is wrong with PHY_E. Everything
> > else seems to be assuming PHY_E including those SNPS_* registers (as
> > you mentioned). I'm starting to think it would be overkill to open up
> > this phy enum for this purpose. I would propose to stick with current
> > patch. Maybe just update commit message. What do you think?
>
> I would put it the other way. It is *only* the SNPS PHY IP registers
> that use the wonky offsets (unless you found some others?). Everythting
> on the Intel IP side wants it to be PHY_F.
>
> So still would make more sense to me to add a new enum for the
> SNPS PHY instance and remap across the boundary. Otherwise we're
> just propagating this madness everwhere rather than containing in
> the SNPS PHY implementation.
Seems people want this is asap. I suppose it'll do as a temporary
measure given the phy stuff is already such mess.
Acked-by: Ville Syrjälä
As for the proper way to do stuff, I'm thinking roughly:
enum intel_spns_phy {
SNPS_PHY_A,
...
SNPS_PHY_TC1, // == current PHY_E in value
};
and I think that can stay entirely inside intel_snps_phy.c.
As for our currnet enum phy I think we could start with something like
this:
enum phy {
PHY_A,
...
PHY_F,
PHY_TC1 = PHY_F,
...
};
I think that should make it line up with PHY_MISC stuff and the
VBT as well. So in the VBT code we could nuke all those crazy mapping
tables and just do:
old platform: port -> VBT port
new platform: phy -> VBT port
And we could probably have encoder->phy which gets populated
in the encoder init per-platform, similar to hpd_pin. That
would get rid of the intel_port_to_phy() disaster.
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, Feb 16, 2022 at 02:11:54PM +, Hogander, Jouni wrote: > On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote: > > On Wed, Feb 16, 2022 at 09:36:02AM +, Hogander, Jouni wrote: > > > On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote: > > > > On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote: > > > > > From: Jouni Högander > > > > > > > > > > Currently ICL_PHY_MISC macro is returning offset 0x64C10 for > > > > > PHY_E > > > > > port. Correct offset is 0x64C14. > > > > > > > > Why is it PHY_E and not PHY_F? > > > > > > This is a valid question. It seems we have followed > > > intel_phy_is_snps() > > > here: > > > > > > // snip > > > else if (IS_DG2(dev_priv)) > > > /* > > >* All four "combo" ports and the TC1 port (PHY E) use > > >* Synopsis PHYs. > > >*/ > > > return phy <= PHY_E; > > > // snip > > > > > > According to spec port E is "No connection". Better place to fix > > > this > > > could be intel_phy_is_snps() itself? > > > > I think the crucial question is where are all the places that > > the results of intel_port_to_phy() get used. > > > > I do see that for all the actual snps phy registers we > > do want PHY_E, but maybe it would be better to have a local > > SNPS_PHY enum just for intel_snps_phy.c, and leave the other > > phy thing for everything else? > > > > Not sure if there is some other register we index with the > > phy that specifically wants PHY_E? > > I went through registers accesses in intel_snps_phy.c. It is actually > only this one register which offset is wrong with PHY_E. Everything > else seems to be assuming PHY_E including those SNPS_* registers (as > you mentioned). I'm starting to think it would be overkill to open up > this phy enum for this purpose. I would propose to stick with current > patch. Maybe just update commit message. What do you think? I would put it the other way. It is *only* the SNPS PHY IP registers that use the wonky offsets (unless you found some others?). Everythting on the Intel IP side wants it to be PHY_F. So still would make more sense to me to add a new enum for the SNPS PHY instance and remap across the boundary. Otherwise we're just propagating this madness everwhere rather than containing in the SNPS PHY implementation. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, 2022-02-16 at 12:07 +0200, Ville Syrjälä wrote: > On Wed, Feb 16, 2022 at 09:36:02AM +, Hogander, Jouni wrote: > > On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote: > > > On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote: > > > > From: Jouni Högander > > > > > > > > Currently ICL_PHY_MISC macro is returning offset 0x64C10 for > > > > PHY_E > > > > port. Correct offset is 0x64C14. > > > > > > Why is it PHY_E and not PHY_F? > > > > This is a valid question. It seems we have followed > > intel_phy_is_snps() > > here: > > > > // snip > > else if (IS_DG2(dev_priv)) > > /* > > * All four "combo" ports and the TC1 port (PHY E) use > > * Synopsis PHYs. > > */ > > return phy <= PHY_E; > > // snip > > > > According to spec port E is "No connection". Better place to fix > > this > > could be intel_phy_is_snps() itself? > > I think the crucial question is where are all the places that > the results of intel_port_to_phy() get used. > > I do see that for all the actual snps phy registers we > do want PHY_E, but maybe it would be better to have a local > SNPS_PHY enum just for intel_snps_phy.c, and leave the other > phy thing for everything else? > > Not sure if there is some other register we index with the > phy that specifically wants PHY_E? I went through registers accesses in intel_snps_phy.c. It is actually only this one register which offset is wrong with PHY_E. Everything else seems to be assuming PHY_E including those SNPS_* registers (as you mentioned). I'm starting to think it would be overkill to open up this phy enum for this purpose. I would propose to stick with current patch. Maybe just update commit message. What do you think? > > Also it kinda looks to me like for VBT port mapping we also > want PHY_F essentially since the modern platforms make the > VBT port mapping PHY based and xelpd_port_mapping() uses > PORT_TC1<->DVO_PORT_*F. Not that we actually use enum phy > in the VBT code atm, but I'm thinking we probably should > since it might allow us to get rid of all those different > mapping tables. Though the whole intel_port_to_phy() > disaster needs to get cleaned up first IMO. > BR, Jouni Högander
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, Feb 16, 2022 at 09:36:02AM +, Hogander, Jouni wrote: > On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote: > > On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote: > > > From: Jouni Högander > > > > > > Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E > > > port. Correct offset is 0x64C14. > > > > Why is it PHY_E and not PHY_F? > > This is a valid question. It seems we have followed intel_phy_is_snps() > here: > > // snip > else if (IS_DG2(dev_priv)) > /* >* All four "combo" ports and the TC1 port (PHY E) use >* Synopsis PHYs. >*/ > return phy <= PHY_E; > // snip > > According to spec port E is "No connection". Better place to fix this > could be intel_phy_is_snps() itself? I think the crucial question is where are all the places that the results of intel_port_to_phy() get used. I do see that for all the actual snps phy registers we do want PHY_E, but maybe it would be better to have a local SNPS_PHY enum just for intel_snps_phy.c, and leave the other phy thing for everything else? Not sure if there is some other register we index with the phy that specifically wants PHY_E? Also it kinda looks to me like for VBT port mapping we also want PHY_F essentially since the modern platforms make the VBT port mapping PHY based and xelpd_port_mapping() uses PORT_TC1<->DVO_PORT_*F. Not that we actually use enum phy in the VBT code atm, but I'm thinking we probably should since it might allow us to get rid of all those different mapping tables. Though the whole intel_port_to_phy() disaster needs to get cleaned up first IMO. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Wed, 2022-02-16 at 10:50 +0200, Ville Syrjälä wrote:
> On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
> > From: Jouni Högander
> >
> > Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E
> > port. Correct offset is 0x64C14.
>
> Why is it PHY_E and not PHY_F?
This is a valid question. It seems we have followed intel_phy_is_snps()
here:
// snip
else if (IS_DG2(dev_priv))
/*
* All four "combo" ports and the TC1 port (PHY E) use
* Synopsis PHYs.
*/
return phy <= PHY_E;
// snip
According to spec port E is "No connection". Better place to fix this
could be intel_phy_is_snps() itself?
>
> > Fix this by handling PHY_E port seprately.
> >
> > Signed-off-by: Matt Roper
> > Signed-off-by: Jouni Högander
> > Signed-off-by: Ramalingam C
> > ---
> > drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 6 --
> > 2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> > b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> > index c60575cb5368..f08061c748b3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> > @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct
> > drm_i915_private *i915)
> > if (!intel_phy_is_snps(i915, phy))
> > continue;
> >
> > - if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
> > + if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy),
> > DG2_PHY_DP_TX_ACK_MASK,
> > 25))
> > drm_err(>drm, "SNPS PHY %c failed to
> > calibrate after 25ms.\n",
> > phy);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 4d12abb2d7ff..354c25f483cb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9559,8 +9559,10 @@ enum skl_power_gate {
> >
> > #define _ICL_PHY_MISC_A0x64C00
> > #define _ICL_PHY_MISC_B0x64C04
> > -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
> > -_ICL_PHY_MISC_B)
> > +#define _DG2_PHY_MISC_TC1 0x64C14 /* TC1="PHY E" but offset as if
> > "PHY F" */
> > +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A,
> > _ICL_PHY_MISC_B)
> > +#define DG2_PHY_MISC(port) ((port) == PHY_E ?
> > _MMIO(_DG2_PHY_MISC_TC1) : \
> > +ICL_PHY_MISC(port))
> > #define ICL_PHY_MISC_MUX_DDID (1 << 28)
> > #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23)
> > #define DG2_PHY_DP_TX_ACK_MASKREG_GENMASK(23,
> > 20)
> > --
> > 2.20.1
BR,
Jouni Högander
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
On Tue, Feb 15, 2022 at 11:21:54AM +0530, Ramalingam C wrote:
> From: Jouni Högander
>
> Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E
> port. Correct offset is 0x64C14.
Why is it PHY_E and not PHY_F?
>
> Fix this by handling PHY_E port seprately.
>
> Signed-off-by: Matt Roper
> Signed-off-by: Jouni Högander
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 6 --
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> index c60575cb5368..f08061c748b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct
> drm_i915_private *i915)
> if (!intel_phy_is_snps(i915, phy))
> continue;
>
> - if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
> + if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy),
> DG2_PHY_DP_TX_ACK_MASK, 25))
> drm_err(>drm, "SNPS PHY %c failed to calibrate
> after 25ms.\n",
> phy);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d12abb2d7ff..354c25f483cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9559,8 +9559,10 @@ enum skl_power_gate {
>
> #define _ICL_PHY_MISC_A 0x64C00
> #define _ICL_PHY_MISC_B 0x64C04
> -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
> - _ICL_PHY_MISC_B)
> +#define _DG2_PHY_MISC_TC10x64C14 /* TC1="PHY E" but offset as if "PHY F"
> */
> +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A,
> _ICL_PHY_MISC_B)
> +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) : \
> + ICL_PHY_MISC(port))
> #define ICL_PHY_MISC_MUX_DDID (1 << 28)
> #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN(1 << 23)
> #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20)
> --
> 2.20.1
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
> -Original Message-
> From: C, Ramalingam
> Sent: Tuesday, February 15, 2022 11:22 AM
> To: intel-gfx ; dri-devel de...@lists.freedesktop.org>
> Cc: Ville Syrjälä ; Shankar, Uma
> ; Hogander, Jouni ; C,
> Ramalingam
> Subject: [PATCH 3/3] drm/i915: Fix for PHY_MISC_TC1 offset
>
> From: Jouni Högander
>
> Currently ICL_PHY_MISC macro is returning offset 0x64C10 for PHY_E port.
> Correct
> offset is 0x64C14.
>
> Fix this by handling PHY_E port seprately.
>
> Signed-off-by: Matt Roper
> Signed-off-by: Jouni Högander
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/display/intel_snps_phy.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 6 --
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> index c60575cb5368..f08061c748b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> @@ -32,7 +32,7 @@ void intel_snps_phy_wait_for_calibration(struct
> drm_i915_private *i915)
> if (!intel_phy_is_snps(i915, phy))
> continue;
>
> - if (intel_de_wait_for_clear(i915, ICL_PHY_MISC(phy),
> + if (intel_de_wait_for_clear(i915, DG2_PHY_MISC(phy),
> DG2_PHY_DP_TX_ACK_MASK, 25))
> drm_err(>drm, "SNPS PHY %c failed to calibrate
> after
> 25ms.\n",
> phy);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d12abb2d7ff..354c25f483cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9559,8 +9559,10 @@ enum skl_power_gate {
>
> #define _ICL_PHY_MISC_A 0x64C00
> #define _ICL_PHY_MISC_B 0x64C04
> -#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A, \
> - _ICL_PHY_MISC_B)
> +#define _DG2_PHY_MISC_TC10x64C14 /* TC1="PHY E" but offset as if "PHY F"
> */
> +#define ICL_PHY_MISC(port) _MMIO_PORT(port, _ICL_PHY_MISC_A,
> _ICL_PHY_MISC_B)
Nit: Align it as was defined earlier to honor line limit.
Looks Good to me.
Reviewed-by: Uma Shankar
> +#define DG2_PHY_MISC(port) ((port) == PHY_E ? _MMIO(_DG2_PHY_MISC_TC1) :
> \
> + ICL_PHY_MISC(port))
> #define ICL_PHY_MISC_MUX_DDID (1 << 28)
> #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN(1 << 23)
> #define DG2_PHY_DP_TX_ACK_MASK REG_GENMASK(23, 20)
> --
> 2.20.1
