On Mon, Nov 30, 2020 at 09:46:18PM -0800, Lucas De Marchi wrote: > On Mon, Nov 30, 2020 at 07:46:39PM +0200, Ville Syrjälä wrote: > >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? > > the second level branch mixing the code path for new and old platform > instead of following the convention we settled on. The convention I care about most is "clear code" which in many cases is best achieved with gmch vs. not split. > But I'm ok with > moving it back as a HAS_* check in the middle of GEN_* check is proving > controversial. > > Lucas De Marchi > > > > >-- > >Ville Syrjälä > >Intel -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
