On Wed, Jan 11, 2023 at 04:39:36PM +0100, Andi Shyti wrote: > Hi Rodrigo, > > On Wed, Jan 11, 2023 at 10:25:56AM -0500, Rodrigo Vivi wrote: > > On Wed, Jan 11, 2023 at 11:44:47AM +0100, Andi Shyti wrote: > > > From: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx> > > > > > > During module load not all the punit transaction have completed > > > and we might end up timing out, as shown by the following > > > warning: > > > > > > i915 0000:4d:00.0: drm_WARN_ON_ONCE(timeout_base_ms > 3) > > > > > > Wait 10 seconds for the punit to settle and complete any > > > outstanding transactions upon module load. > > > > 10 *SECONDS* ?! > > Don't be alarmed :) > > It's up to 10 seconds, otherwise we would end up waiting up to 3 > minutes. > > And I've seen a version (and you did as well) where those 3 > minutes were raised to 6 (for the PVC particular case). Oh yeap! and that case is funny! Because the indication from PCODE is that 10 seconds is enough, but there are some rare cases where it gets stuck and end up taking a very long time. Then they multiplied the bad rare case to 3, and no idea why how that become 6. But anyway, thanks for refreshing my memory. When I first noticed this patch I thought it was in all the platforms, where this wouldn't make sense. But on discrete where the pcode needs to bring the memory and gt up before we can really use it, it makes sense. And then I noticed that your patch is indeed for dgfx only, so it would be okay. And 10 seconds is okay. However I also noticed that you do this before the other pcode_init check that we were told by pcode folks to use. So, I don't understand how your patch is helping now... you wait for 10 seconds and then you will wait more 10 seconds on the pcode_ready... why the pcode_ready check that we have in case already doesn't cover yours? And why that return? > > Thanks for checking this, > Andi > > > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7814 > > > > > > Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx> > > > Co-developed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/intel_pcode.c | 35 ++++++++++++++++++++++++++---- > > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pcode.c b/drivers/gpu/drm/i915/intel_pcode.c > > > index a234d9b4ed14..3db2ba439bb5 100644 > > > --- a/drivers/gpu/drm/i915/intel_pcode.c > > > +++ b/drivers/gpu/drm/i915/intel_pcode.c > > > @@ -204,15 +204,42 @@ int skl_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request, > > > #undef COND > > > } > > > > > > +static int pcode_init_wait(struct intel_uncore *uncore, int timeout_ms) > > > +{ > > > + if (__intel_wait_for_register_fw(uncore, > > > + GEN6_PCODE_MAILBOX, > > > + GEN6_PCODE_READY, 0, > > > + 500, timeout_ms, > > > + NULL)) > > > + return -EPROBE_DEFER; > > > + > > > + return skl_pcode_request(uncore, > > > + DG1_PCODE_STATUS, > > > + DG1_UNCORE_GET_INIT_STATUS, > > > + DG1_UNCORE_INIT_STATUS_COMPLETE, > > > + DG1_UNCORE_INIT_STATUS_COMPLETE, timeout_ms); > > > +} > > > + > > > int intel_pcode_init(struct intel_uncore *uncore) > > > { > > > + int err; > > > + > > > if (!IS_DGFX(uncore->i915)) > > > return 0; > > > > > > - return skl_pcode_request(uncore, DG1_PCODE_STATUS, > > > - DG1_UNCORE_GET_INIT_STATUS, > > > - DG1_UNCORE_INIT_STATUS_COMPLETE, > > > - DG1_UNCORE_INIT_STATUS_COMPLETE, 180000); > > > + /* > > > + * Wait 10 seconds so that the punit to settle and complete > > > + * any outstanding transactions upon module load > > > + */ > > > + err = pcode_init_wait(uncore, 10000); > > > + > > > + if (err) { > > > + drm_notice(&uncore->i915->drm, > > > + "Waiting for HW initialisation...\n"); > > > + err = pcode_init_wait(uncore, 180000); > > > + } > > > + > > > + return err; > > > } > > > > > > int snb_pcode_read_p(struct intel_uncore *uncore, u32 mbcmd, u32 p1, u32 p2, u32 *val) > > > -- > > > 2.34.1 > > >
