On Tue, Feb 01, 2022 at 09:41:33AM -0800, Lucas De Marchi wrote: > On Tue, Feb 01, 2022 at 09:12:05AM -0800, Jose Souza wrote: > > On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote: > > > Only x86 and in some cases PPC have support added in drm_cache.c for the > > > clflush class of functions. However warning once is sufficient to taint > > > the log instead of spamming it with "Architecture has no drm_cache.c > > > support" every few millisecond. Switch to WARN_ONCE() so we still get > > > the log message, but only once, together with the warning. E.g: > > > > > > ------------[ cut here ]------------ > > > Architecture has no drm_cache.c support > > > WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] > > > ... > > > > > > v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err() > > > > Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > > But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? > > Anyways, it is better than before. > > I thought about that, but it didn't seem justifiable because: > > 1) drm_WARN_ONCE will basically add dev_driver_string() to the log. > However the warning message here is basically helping the bootstrap of > additional archs. They shouldn't be seen on anything properly supported. > > 2) This seems all to be a layer below drm anyway and could even be used > in places outside easy access to a drm pointer. > > So, it seems the benefit of using the subsystem-specific drm_WARN_ONCE > doesn't justify the hassle of changing the callers, possibly adding > additional back pointers to have access to the drm device pointer. Initially I had same feeling as Jose, but good points raised here. Acked-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > thanks > Lucas De Marchi
