El lun, 12-01-2026 a las 07:53 -0300, Maíra Canal escribió:
> Hi Iago,
>
> On 12/01/26 04:57, Iago Toral wrote:
> > El vie, 09-01-2026 a las 15:35 -0300, Maíra Canal escribió:
> > (...)
> > > @@ -126,9 +126,9 @@ v3d_reset(struct v3d_dev *v3d)
> > > {
> > > struct drm_device *dev = &v3d->drm;
> > >
> > > - DRM_DEV_ERROR(dev->dev, "Resetting GPU for hang.\n");
> > > - DRM_DEV_ERROR(dev->dev, "V3D_ERR_STAT: 0x%08x\n",
> > > - V3D_CORE_READ(0, V3D_ERR_STAT));
> > > + drm_warn(dev, "Resetting GPU for hang.\n");
> > > + drm_warn(dev, "V3D_ERR_STAT: 0x%08x\n", V3D_CORE_READ(0,
> > > V3D_ERR_STAT));
> > > +
> >
> > These look like they should be drm_err, no?
>
> Actually, I decided to change to a warning as a GPU reset is not
> always
> an error. For example, we have the DRM_GPU_SCHED_STAT_NO_HANG
> scenario,
> in which the GPU isn't actually hung.
But we only use that if we are not actually resetting the GPU, no? the
messages above are for when we actually reset.
> Also, sometimes we have GPU resets
> that recover the GPU and the user doesn't see any disruption.
>
In the event of a reset there is always going to be at least a
performance hiccup which would be noticeable by users, but more
importantly, unless we are deciding to reset the GPU for some internal
reason unrelated to the user (I don't think we ever do this?) it signs
that something has gone really wrong, so making it an explicit error
message makes more sense to me.
> I believe a warning is a good compromise between an error and a debug
> message, but I'm fine with changing it back to an error if you
> believe
> it's more suitable.
>
> >
> > (...)
> >
> >
> > > static int
> > > -v3d_get_multisync_post_deps(struct drm_file *file_priv,
> > > +v3d_get_multisync_post_deps(struct v3d_dev *v3d,
> >
> > Same comment as in the previous patch. Again, not necessarily
> > against
> > it.
>
> I'll address it in v2. Thanks!
>
> >
> > > + struct drm_file *file_priv,
> > > struct v3d_submit_ext *se,
> > > u32 count, u64 handles)
> > > {
> > > @@ -346,7 +347,7 @@ v3d_get_multisync_post_deps(struct drm_file
> > > *file_priv,
> > >
> > > if (copy_from_user(&out, post_deps++,
> > > sizeof(out)))
> > > {
> > > ret = -EFAULT;
> > > - DRM_DEBUG("Failed to copy post dep
> > > handles\n");
> > > + drm_dbg(&v3d->drm, "Failed to copy post
> > > dep
> > > handles\n");
> >
> > I think we have a lot of these kind of things as dbg that sould
> > probably be err, no? Not sure if that is very relevant though, but
> > we
> > can fix that separately if we want to.
>
> I have the impression that those messages are more useful for us,
> developers, than the end user. For such reason, I believe that a
> debug
> message is more suitable, as the userspace will (hopefully) handle
> the
> errno gracefully and in case of a bug, we can use ``drm.debug`` to
> investigate it without swamping the user's kernel log.
I think many of these would only happen in OOM scenarios, at which
point swamping the user's kernel log is probably the least of their
concerns... but in any case, I think these would be rare enough that is
not a big deal if we dump them as debug or error messages, so I am okay
with keeping this as-is.
Iago
>
> Best regards,
> - Maíra
>
> >
> > Iago
> >
> > > goto fail;
> > > }
> > >
>
>
