Re: [PATCH 5.10 1/2] drm/i915/dg1: Wait for pcode/uncore handshake at startup

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 14 Jun 2023 17:17:35 -0700
schrieb Matt Roper <matthew.d.roper@intel.com>:

> On Mon, Jun 12, 2023 at 09:30:30AM +0200, Henning Schild wrote:
> > Am Wed, 7 Jun 2023 20:09:58 +0200
> > schrieb Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> >   
> > > On Fri, Jun 02, 2023 at 06:05:06PM +0200, Henning Schild wrote:  
> > > > From: Matt Roper <matthew.d.roper@intel.com>
> > > > 
> > > > From: Matt Roper <matthew.d.roper@intel.com>    
> > > 
> > > Twice?
> > >   
> > > > 
> > > > [ Upstream commit f9c730ede7d3f40900cb493890d94d868ff2f00f ]
> > > > 
> > > > DG1 does some additional pcode/uncore handshaking at
> > > > boot time; this handshaking must complete before various other
> > > > pcode commands are effective and before general work is
> > > > submitted to the GPU. We need to poll a new pcode mailbox
> > > > during startup until it reports that this handshaking is
> > > > complete.
> > > > 
> > > > The bspec doesn't give guidance on how long we may need to wait
> > > > for this handshaking to complete.  For now, let's just set a
> > > > really long timeout; if we still don't get a completion status
> > > > by the end of that timeout, we'll just continue on and hope for
> > > > the best.
> > > > 
> > > > v2 (Lucas): Rename macros to make clear the relation between
> > > > command and result (requested by José)
> > > > 
> > > > Bspec: 52065
> > > > Cc: Clinton Taylor <Clinton.A.Taylor@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> > > > Link:
> > > > https://patchwork.freedesktop.org/patch/msgid/20201001063917.3133475-2-lucas.demarchi@intel.com
> > > >    
> > > 
> > > You also need to sign-off on a patch you submit for inclusion
> > > anywhere, right?  
> > 
> > I was not sure that was needed for a backport, but will add it once
> > i resend. 
> >   
> > > Please resend this series with that added so that we can queue
> > > them up.  
> > 
> > Will do.
> > 
> > Matt would you agree? As i said i just googled/bisected and found
> > this one and it seems to help. But you seem to say that it does not
> > fit. I am guessing the patch might not be as atomic as could be,
> > that is why backporting it helps.  
> 
> Sorry for the slow response; I've been traveling and am just catching
> up on email now.
> 
> Since you're running on a platform with integrated graphics, this
> patch can't have any functional impact.  The function added in this
> patch only does something on discrete GPU platforms; on all others it
> bails out immediately:
> 
>         +       if (!IS_DGFX(i915))
>         +               return;
> 
> The only Intel discrete devices that return true from IS_DGFX are DG1,
> DG2, and PVC, none of which were supported yet on the 5.10 kernel.
> 
> The dmesg splat you pasted in your cover letter is coming from the
> DRAM detection code, which is what the other patch in your series
> ("drm/i915/gen11+: Only load DRAM information from pcode") is dealing
> with.  So I think that other patch is the only one you should need;
> this pcode one isn't having any effect.

Thanks for explaining that Matt. This patch was just backported because
it introduced

void intel_pcode_init(struct drm_i915_private *i915)

which is needed to apply the patch 2 (the one i really care about)
without modifications.

I have two options now, still cherry-pick both like i did here. Where
we accept the fact that p1 is essentially a no-op. (1) Or i remove the
call to intel_pcode_init from p2 as i cherry-pick that. (2)

IMHO option 1 is better because p2 will apply and compile without
modification. So i will do that again, fix the double-From problems and
include a message why p1 is needed into the cover-letter.

regards,
Henning

> 
> Matt
> 
> > 
> > Henning
> >   
> > > thanks,
> > > 
> > > greg k-h  
> >   
>