Re: [Intel-gfx] [PATCH 3/3] drm/i915/display: remove strap checks from gen 9

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Tue, Apr 13, 2021 at 08:39:07PM +0300, Ville Syrjälä wrote:
>On Tue, Apr 13, 2021 at 10:22:24AM -0700, Lucas De Marchi wrote:
>> On Tue, Apr 13, 2021 at 06:45:16PM +0300, Ville Syrjälä wrote:
>> >On Mon, Apr 12, 2021 at 11:09:27PM -0700, Lucas De Marchi wrote:
>> >> Direction on gen9+ was to stop reading the straps and only rely on the
>> >> VBT for marking the port presence. This happened while dealing with
>> >> WaIgnoreDDIAStrap and instead of using it as a WA, it should now be the
>> >> normal flow. See commit 885d3e5b6f08 ("drm/i915/display: fix comment on
>> >> skl straps").
>> >>
>> >> For gen 10 it's hard to say if this will work or not since I can't test
>> >> it, so leave it with the same behavior as before.
>> >>
>> >> For PCH_TGP we should still rely on the VBT to make ports E and F not
>> >> available.
>> >>
>> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> Reviewed-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_display.c | 36 ++++++--------------
>> >>  1 file changed, 11 insertions(+), 25 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> >> index d62ce9c87748..5a03cbba0280 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> >> @@ -10883,34 +10883,25 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>> >>  		intel_ddi_init(dev_priv, PORT_B);
>> >>  		intel_ddi_init(dev_priv, PORT_C);
>> >>  		vlv_dsi_init(dev_priv);
>> >> +	} else if (DISPLAY_VER(dev_priv) == 9) {
>> >
>> >Should be >=10 I presume? Or did we want ot handle cnl along with
>>
>> why >= 10? The only DISPLAY_VER() == 10 platforms out there are handled
>> in the branch above. I can make it >= 9, but not >= 10. Intention was to
>> handle skl/kbl here.
>
>Yeah, meant to write >=9. Cnl not really a thing, but I would get
>confused if we started skipping it in some places while still
>handling it in others. I guess we may want to consider just nuking
>cnl totally everywhere, but until that time I think we should keep
>things consistent.

considering mesa already did that, then yes. And agreed about being
consistent while that doesn't happen.

thanks
Lucas De Marchi

>
>>
>>
>> >icl perhaps? Doesn't really matter I suppose, but it's surely
>> >going to consfuse the me the next time I read this.
>> >
>> >> +		intel_ddi_init(dev_priv, PORT_A);
>> >> +		intel_ddi_init(dev_priv, PORT_B);
>> >> +		intel_ddi_init(dev_priv, PORT_C);
>> >> +		intel_ddi_init(dev_priv, PORT_D);
>> >> +		intel_ddi_init(dev_priv, PORT_E);
>> >> +		intel_ddi_init(dev_priv, PORT_F);
>> >
>> >DDI F isn't a thing on skl/derivatives, so I'd probably skip it on
>> >those. Could just use IS_CNL_WITH_PORT_F() to match the looks of
>> >the icl stuff.
>>
>> I was actually looking at ICL and thinking "shouldn't this hack for
>> broken VBT be hidden in intel_bios.c?"  I think we should trust what we
>> parse from VBT everywhere  except of course in intel_bios.c where we
>> fixup when the VBT is wrong. Thoughts?
>
>I guess we could stuff it all in there somehow. Not sure.
>Maybe Jani has thoughts on this?
>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx