Re: [PATCH v3] drm/i915/xe3lpd: Power request asserting/deasserting

[Raag Jadav <raag.jadav@intel.com>]

On Wed, Oct 30, 2024 at 04:52:20PM +0530, Kahola, Mika wrote:

...

> > > > +static void wa_14020908590(struct intel_display *display,
> > > > +			   bool enable)
> > > > +{
> > > > +	bool error = false;
> > > > +
> > > > +	/* check if mailbox is running busy */
> > > > +	if (intel_de_wait_for_clear(display, TCSS_DISP_MAILBOX_IN_CMD,
> > > > +				    TCSS_DISP_MAILBOX_IN_CMD_RUN_BUSY,
> > 10)) {
> > > > +		drm_dbg_kms(display->drm,
> > > > +			    "Timeout waiting for TCSS mailbox run/busy bit to
> > clear\n");
> > >
> > > Just do drm_WARN() with the message here.
> > 
> > Rather,
> > 
> > 	ret = intel_de_wait_for_clear();
> > 	if (drm_WARN(drm, ret, ...))
> > 		return;
> > 
> > Cleaner?
> 
> Maybe we could drop the drm_WARN_ON() completely? This is something that
> we are not really using elsewhere in the driver. Simply drm_dbg_kms() on
> timeouts has so far been enough. What do you think?

Right, WARN() is usually for cases that *should never happen* or have *serious
consequences*. Unless that's the case, I'm not sure if it'll be useful here.

Raag