Re: [Intel-gfx] [PATCH v3] drm/i915/gt: move remaining debugfs interfaces into gt

[Andi Shyti <andi@etezian.org>]

Hi Ville,

> > +	if (IS_CHERRYVIEW(i915)) {
> > +		seq_printf(m, "Master Interrupt Control:\t%08x\n",
> > +			   intel_uncore_read(uncore, GEN8_MASTER_IRQ));
> > +
> > +		for (i = 0; i < 4; i++) {
> > +			seq_printf(m, "GT Interrupt IMR %d:\t%08x\n",
> > +				   i, intel_uncore_read(uncore,
> > +							GEN8_GT_IMR(i)));
> > +			seq_printf(m, "GT Interrupt IIR %d:\t%08x\n",
> > +				   i, intel_uncore_read(uncore,
> > +							GEN8_GT_IIR(i)));
> > +			seq_printf(m, "GT Interrupt IER %d:\t%08x\n",
> > +				   i, intel_uncore_read(uncore,
> > +							GEN8_GT_IER(i)));
> > +		}
> > +
> 
> Why does chv get its own block? Looks identical to the bdw block.

[...]

> > +	} else if (!HAS_PCH_SPLIT(i915)) {
> 
> HAS_PCH_SPLIT in gt code looks a bit out of place.
> 
> > +		seq_printf(m, "Interrupt enable:    %08x\n",
> > +			   intel_uncore_read(uncore, GEN2_IER));
> > +		seq_printf(m, "Interrupt identity:  %08x\n",
> > +			   intel_uncore_read(uncore, GEN2_IIR));
> > +		seq_printf(m, "Interrupt mask:      %08x\n",
> > +			   intel_uncore_read(uncore, GEN2_IMR));
> 
> Technically on gen2 these are 16 bits, on gen3+ 32 bits. I don't
> quite remember now what happens when you read them as 32bit on gen2.
> Might be the msbs just come out all zeroes.
> 
> > +	} else {
> > +		seq_printf(m, "Graphics Interrupt enable:		%08x\n",
> > +			   intel_uncore_read(uncore, GTIER));
> > +		seq_printf(m, "Graphics Interrupt identity:		%08x\n",
> > +			   intel_uncore_read(uncore, GTIIR));
> > +		seq_printf(m, "Graphics Interrupt mask:		%08x\n",
> > +			   intel_uncore_read(uncore, GTIMR));
> 
> Probably this should be before the previous block to keep things in
> order. Also missing GEN6_PM stuff for snb+, and the master interrupt
> stuff for ilk+.

[...]

thanks for your input. Actually I'm not even sure this part of
the patch is required anymore. It was originally a copy paste of
the parts of the 'i915_interrupt_info' functions in the
'i915_gem_interrupt' debugfs file.

This file was removed here by Chris:

	cf977e18610e6 ("drm/i915/gem: Spring clean debugfs")

I have reproposed it in this patch seeking for comments because I
wanted to understand after all this time it might be needed or
not. If not I was going to remove it in my last version.

I haven't received any comments and therefore I am going to
remove it in v4.

Thank you,
Andi