On Fri, 01 Jul 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Jul 01, 2022 at 12:55:43PM +0300, Jani Nikula wrote:
>> On Thu, 30 Jun 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> >
>> > MCC is derived from TGP, and we have no real need to
>> > differentiate between the two. Thus remove PCH_MCC and
>> > just declare it to be PCH_TGP compatible.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
>> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
>> > drivers/gpu/drm/i915/intel_pch.c | 3 ++-
>> > drivers/gpu/drm/i915/intel_pch.h | 4 +---
>> > 4 files changed, 5 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > index 272e1bf6006b..2330604b0bcc 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> > @@ -4179,7 +4179,7 @@ static enum hpd_pin ehl_hpd_pin(struct drm_i915_private *dev_priv,
>> > if (port == PORT_D)
>> > return HPD_PORT_A;
>> >
>> > - if (HAS_PCH_MCC(dev_priv))
>> > + if (HAS_PCH_TGP(dev_priv))
>> > return icl_hpd_pin(dev_priv, port);
>> >
>> > return HPD_PORT_A + port - PORT_A;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > index 1ae09431f53a..ebd91aa69dd2 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > @@ -2852,7 +2852,7 @@ static u8 intel_hdmi_ddc_pin(struct intel_encoder *encoder)
>> > ddc_pin = rkl_port_to_ddc_pin(dev_priv, port);
>> > else if (DISPLAY_VER(dev_priv) == 9 && HAS_PCH_TGP(dev_priv))
>> > ddc_pin = gen9bc_tgp_port_to_ddc_pin(dev_priv, port);
>> > - else if (HAS_PCH_MCC(dev_priv))
>> > + else if (IS_JSL_EHL(dev_priv) && HAS_PCH_TGP(dev_priv))
>> > ddc_pin = mcc_port_to_ddc_pin(dev_priv, port);
>>
>> Nitpick, mcc_ prefix is now an outlier here, and could be named after
>> the CPU/PCH combo like above for gen 9 and TGP. But no big deal.
>
> I want to rewrite these entirely. They should be doing 99% the same
> thing as the foo_hpd_pin() functions, yet they are written in all
> various kinds of different ways (none of which match the hpd_pin()
> stuff).
>
> Also while looking at that I stumbled on the VBT code doing a
> slightly different variant of the same stuff using arrays. And on
> top of that we have the VBT AUX CH mapping code as well, written
> in yet another style. So I think I want to try to unify it all
> to a common approach.
Ack. I've looked at all of this before, but haven't really come up with
anything neat, just grumbling about how it's split between VBT parsing
and this.
> I'm thinking the array approach might be the easiest to parse for
> mere mortals, so kinda leaning towards that. What do you think?
Sounds good, and it has a better separation of platforms, instead of a
bunch of magic conditions. OTOH I'm not overly enthusiastic about how
dvo_port_to_port() ended up looking, and why it needs to be platform
specific at all, oh well. I think you'll only know for real when you've
played with the code and have half the implementation written...
> Although one hurdle between me and arrays for the VBT AUX CH stuff
> is the VBT values which are shifted up into the upper nibble of
> the byte. So using those directly would result in giant arrays
> which are mostly empty. But I could redefine the VBT values as
> just the upper four bits and shift down when parsing the VBT.
If we can hide that in intel_bios.c or something, should be fine I
think.
BR,
Jani.
>
>>
>> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> Thanks.
>
>>
>>
>>
>> > else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>> > ddc_pin = icl_port_to_ddc_pin(dev_priv, port);
>> > diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
>> > index 94446cac6605..b45c504c6f03 100644
>> > --- a/drivers/gpu/drm/i915/intel_pch.c
>> > +++ b/drivers/gpu/drm/i915/intel_pch.c
>> > @@ -116,7 +116,8 @@ intel_pch_type(const struct drm_i915_private *dev_priv, unsigned short id)
>> > case INTEL_PCH_MCC_DEVICE_ID_TYPE:
>> > drm_dbg_kms(&dev_priv->drm, "Found Mule Creek Canyon PCH\n");
>> > drm_WARN_ON(&dev_priv->drm, !IS_JSL_EHL(dev_priv));
>> > - return PCH_MCC;
>> > + /* MCC is TGP compatible */
>> > + return PCH_TGP;
>> > case INTEL_PCH_TGP_DEVICE_ID_TYPE:
>> > case INTEL_PCH_TGP2_DEVICE_ID_TYPE:
>> > drm_dbg_kms(&dev_priv->drm, "Found Tiger Lake LP PCH\n");
>> > diff --git a/drivers/gpu/drm/i915/intel_pch.h b/drivers/gpu/drm/i915/intel_pch.h
>> > index b7a8cf409d48..07f6f5517968 100644
>> > --- a/drivers/gpu/drm/i915/intel_pch.h
>> > +++ b/drivers/gpu/drm/i915/intel_pch.h
>> > @@ -24,8 +24,7 @@ enum intel_pch {
>> > PCH_CNP, /* Cannon/Comet Lake PCH */
>> > PCH_ICP, /* Ice Lake PCH */
>> > PCH_JSP, /* Jasper Lake PCH */
>> > - PCH_MCC, /* Mule Creek Canyon PCH */
>> > - PCH_TGP, /* Tiger Lake PCH */
>> > + PCH_TGP, /* Tiger Lake/Mule Creek Canyon PCH */
>> > PCH_ADP, /* Alder Lake PCH */
>> >
>> > /* Fake PCHs, functionality handled on the same PCI dev */
>> > @@ -69,7 +68,6 @@ enum intel_pch {
>> > #define HAS_PCH_ADP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)
>> > #define HAS_PCH_DG1(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_DG1)
>> > #define HAS_PCH_JSP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_JSP)
>> > -#define HAS_PCH_MCC(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_MCC)
>> > #define HAS_PCH_TGP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_TGP)
>> > #define HAS_PCH_ICP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_ICP)
>> > #define HAS_PCH_CNP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_CNP)
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
