All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: ville.syrjala@linux.intel.com, dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org, Emil Velikov <emil.l.velikov@gmail.com>
Subject: Re: [Intel-gfx] [PATCH 1/8] drm: Add crtc->name and use it in debug	messages
Date: Fri, 13 Nov 2015 12:18:24 +0200	[thread overview]
Message-ID: <87vb9619u7.fsf@intel.com> (raw)
In-Reply-To: <1447357144-1172-2-git-send-email-ville.syrjala@linux.intel.com>

On Thu, 12 Nov 2015, ville.syrjala@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);

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

If you are all "meh" I guess I can live with your approach too.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2015-11-13 10:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 19:38 [PATCH v2 0/8] drm: Give crtcs and planes actual names (v2) ville.syrjala
2015-11-12 19:38 ` [PATCH 1/8] drm: Add crtc->name and use it in debug messages ville.syrjala
2015-11-13 10:18   ` Jani Nikula [this message]
2015-11-13 11:03     ` [Intel-gfx] " Ville Syrjälä
2015-11-17 10:15       ` Daniel Vetter
2015-11-17 11:20         ` Ville Syrjälä
2015-11-12 19:38 ` [PATCH 2/8] drm: Add plane->name and use it in debug prints ville.syrjala
2015-11-12 19:38 ` [PATCH 3/8] drm/i915: Use crtc->name in debug messages ville.syrjala
2015-11-12 19:39 ` [PATCH 4/8] drm/i915: Use plane->name in debug prints ville.syrjala
2015-11-12 19:39 ` [PATCH v2 5/8] drm/i915: Set crtc->name to "pipe A", "pipe B", etc ville.syrjala
2015-11-12 19:39 ` [PATCH 6/8] drm/i915: Fix plane init failure paths ville.syrjala
2015-11-12 19:39 ` [PATCH 7/8] drm/i915: Don't leak primary/cursor planes on crtc init failure ville.syrjala
2015-11-12 19:39 ` [PATCH v2 8/8] drm/i915: Give meaningful names to all the planes ville.syrjala

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87vb9619u7.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.