[Intel-gfx] [PATCH 1/8] drm: Add crtc->name and use it in debug messages

[Ville Syrjälä]

On Fri, Nov 13, 2015 at 12:18:24PM +0200, Jani Nikula wrote:
> On Thu, 12 Nov 2015, ville.syrjala at linux.intel.com wrote:
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 24c5434..ea00a69 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -677,6 +677,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> >  	crtc->dev = dev;
> >  	crtc->funcs = funcs;
> >  
> > +	if (!crtc->name)
> > +		crtc->name = "";
> > +
> 
> This, and the cleanup dance you have to do in patch 5/8, are my only
> gripes with the series. I would prefer it if crtc->name and plane->name
> were managed dynamically by the init/cleanup functions like
> connector->name and encoder->name are. However those are easier because
> the caller doesn't decide the name.
> 
> Do you think it would be too ugly to have this here:
> 
> 	crtc->name = kstrdup(crtc->name ? crtc->name : "", GFP_KERNEL);

Hmm. Yeah that might be a decentish alternative. I initialliy tried to
do dynamic allocation in both the driver and core, and that made the
error handling during init rather nasty. But if it's only the core that
does it, it might be OK.

> 
> It would of course be neater if the name was passed in as parameter, but
> that would generate quite a bit of unnecessary churn.

Yeah, I've been hitting a lot of drivers recently with a bunch of stuff,
and was sort of hoping to avoid it this time. But if people prefer a
passed in parameter, I could do it as well. Maybe this time I could even
get coccinelle to cooperate with me (whether that happens seems to depend
on the phase of the moon or something).

> 
> If you are all "meh" I guess I can live with your approach too.
> 
> BR,
> Jani.
> 
> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC