Re: [Intel-gfx] [PATCH 1/3] drm/i915: Make backlight setup debugs consistent

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 15, 2023 at 12:14:59PM +0200, Jani Nikula wrote:
> On Tue, 14 Feb 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > It's confusing to debug backlight issues when one can't
> > easily even tell what kind of backlight control was
> > selected. Sprinkle uniform debug messages to all the
> > backlight setup functions.
> >
> > Also the one that was already there (ext_pwm) was
> > using drm_info() for some reason. I don't think that's
> > warranted so switch it to drm_dbg_kms() as well.
> 
> Maybe add similar in intel_dp_aux_backlight.c to the two setup functions
> there?

Forgot about that one. But I think that one also calls the
pwm setup function so might need to think about the ordering
a bit so that the log makes at least some sense.

> Also the message in intel_backlight_setup() could use some
> tweaking.

I'll take a gander at it.

> 
> > @@ -1392,6 +1408,10 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe
> >  
> >  	panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
> >  
> > +	drm_dbg_kms(&i915->drm,
> > +		    "[CONNECTOR:%d:%s] Using native PWM for backlight control\n",
> > +		    connector->base.base.id, connector->base.name);
> 
> Maybe log the pipe here similar to controller in bxt/cnp?

In this case it's just the initial pipe. But I guess we 
could log it. Looks like we have a debug message for it
already in intel_edp_backlight_setup(). I suppose I could
remove that one and include the information here instead.

> 
> Otherwise, LGTM.
> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel