[Intel-gfx] [PATCH] Revert "drm/i915: re-order if/else ladder for hpd_irq_setup"

[Ville Syrjälä]

On Mon, Nov 30, 2020 at 09:31:04AM -0800, Lucas De Marchi wrote:
> On Mon, Nov 30, 2020 at 04:19:54PM +0200, Ville Syrjälä wrote:
> >On Fri, Nov 27, 2020 at 08:52:29PM -0800, Lucas De Marchi wrote:
> >> On Fri, Nov 27, 2020 at 02:57:48PM +0000, Chris Wilson wrote:
> >> >We now use ilk_hpd_irq_setup for all GMCH platforms that do not have
> >> >hotplug. These are early gen3 and gen2 devices that now explode on boot
> >> >as they try to access non-existent registers.
> >>
> >> humn... true, my bad. But I don't think a revert is the right fix. It
> >> would be much better if we would not be setting up the hpd setup
> >> function at all for platforms that do not have hotplug. I think a
> >> separate early check for I915_HAS_HOTPLUG() would be deserved.
> >
> >I think it generally leads to much less convoluted logic when we keep
> >gmch vs. rest separate. So I'm confused as to what we're even trying
> >to achieve here?
> 
> 1) Stop trying to setup hotplug in a platform that doesn't have hotplug
> was the main focus. Later it would be better to move some of these
> hotplug to display/  as they are clearly display related and account for
> a great portion of i915_irq.c.
> 
> I left the I915_HAS_HOTPLUG() in the middle by
> mistake, it should had been an earlier call.
> 
> 2) semi-related is the move of GMCH to the middle and I guess this is
> what you're complaining here. I find it's cumbersome to have it
> separate as we go and extend these checks for newer platforms. Almost
> everywhere we settled on having last platform first in the if/else
> ladders - this makes it much more clear on how/where to add a new
> platform.

You never touch the gmch path for new platforms. What could be more
clear than that?

-- 
Ville Syrjälä
Intel