Re: [PATCH 11/12] drm/client: Streamline mode selection debugs

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

On Fri, Apr 05, 2024 at 09:57:07AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 04.04.24 um 22:33 schrieb Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Get rid of all the redundant debugs and just wait until the end
> > to print which mode (and of which type) we picked.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_client_modeset.c | 65 +++++++++++++---------------
> >   1 file changed, 31 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > index 415d1799337b..ad88c11037d8 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -408,6 +408,8 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> >   
> >   retry:
> >   	for (i = 0; i < connector_count; i++) {
> > +		const char *mode_type;
> > +
> >   		connector = connectors[i];
> >   
> >   		if (conn_configured & BIT_ULL(i))
> > @@ -440,20 +442,20 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> >   			drm_client_get_tile_offsets(dev, connectors, connector_count, modes, offsets, i,
> >   						    connector->tile_h_loc, connector->tile_v_loc);
> >   		}
> > -		drm_dbg_kms(dev, "looking for cmdline mode on [CONNECTOR:%d:%s]\n",
> > -			    connector->base.id, connector->name);
> >   
> > -		/* got for command line mode first */
> > +		mode_type = "cmdline";
> >   		modes[i] = drm_connector_pick_cmdline_mode(connector);
> > +
> >   		if (!modes[i]) {
> > -			drm_dbg_kms(dev, "looking for preferred mode on [CONNECTOR:%d:%s] (tile group: %d)\n",
> > -				    connector->base.id, connector->name,
> > -				    connector->tile_group ? connector->tile_group->id : 0);
> > +			mode_type = "preferred";
> >   			modes[i] = drm_connector_preferred_mode(connector, width, height);
> >   		}
> > -		/* No preferred modes, pick one off the list */
> > -		if (!modes[i])
> > +
> > +		if (!modes[i]) {
> > +			mode_type = "first";
> >   			modes[i] = drm_connector_first_mode(connector);
> > +		}
> > +
> >   		/*
> >   		 * In case of tiled mode if all tiles not present fallback to
> >   		 * first available non tiled mode.
> > @@ -468,16 +470,20 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> >   			    (connector->tile_h_loc == 0 &&
> >   			     connector->tile_v_loc == 0 &&
> >   			     !drm_connector_get_tiled_mode(connector))) {
> > -				drm_dbg_kms(dev, "Falling back to non tiled mode on [CONNECTOR:%d:%s]\n",
> > -					    connector->base.id, connector->name);
> > +				mode_type = "non tiled";
> >   				modes[i] = drm_connector_fallback_non_tiled_mode(connector);
> >   			} else {
> > +				mode_type = "tiled";
> >   				modes[i] = drm_connector_get_tiled_mode(connector);
> >   			}
> >   		}
> >   
> > -		drm_dbg_kms(dev, "found mode %s\n",
> > -			    modes[i] ? modes[i]->name : "none");
> > +		if (!modes[i])
> > +			mode_type = "no";
> > +
> > +		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found %s mode: %s\n",
> > +			    connector->base.id, connector->name,
> > +			    mode_type, modes[i] ? modes[i]->name : "none");
> 
> Instead of tracking the whole mode_type thing, maybe just do
> 
> if (!modes[i])
>      drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found mode: " DRM_MODE_FMT, 
> DRM_MODE_ARG(modes[i]) );
> 
> to print the full mode.

The point of the mode_type is to indicate how we derived 
that mode. Printing the full modeline doesn't help with that.

-- 
Ville Syrjälä
Intel