Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jani Nikula <jani.nikula@linux.intel.com>
To:     Sam Ravnborg <sam@ravnborg.org>,
        Thierry Reding <thierry.reding@gmail.com>
Cc:     Stefan Mavrodiev <stefan@olimex.com>,
        David Airlie <airlied@linux.ie>, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, Sean Paul <sean@poorly.run>
Subject: Re: [PATCH v1 0/19] drm/panel: drmP.h removal and DRM_DEV*
In-Reply-To: <20190201092037.GA9251@ravnborg.org>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
References: <20190131192619.9763-1-sam@ravnborg.org> <20190131200723.GZ114153@art_vandelay> <20190131210312.GA8947@ravnborg.org> <20190131215417.GA13156@mithrandir> <20190201092037.GA9251@ravnborg.org>
Date:   Fri, 01 Feb 2019 12:30:10 +0200
Message-ID: <87tvhnyf65.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 01 Feb 2019, Sam Ravnborg <sam@ravnborg.org> wrote:
> Hi Thierry.
>
>> 
>> I personally like the DRM_DEV_* variants better because of the
>> additional information that they provide. That can be useful when
>> grepping logs etc.
>> 
>> I'm slightly on the fence about this patch. The unwritten, and
>> admittedly fuzzy, rules that I've been using so far are that dev_*() are
>> used or messages that have to do with the panel device itself, whereas
>> DRM_* variants are used for things that are actually related to DRM. So
>> typically this would mean that roughly everything in ->probe() or
>> ->remove() would be dev_*(), while the rest would be DRM_DEV_*().
>
> For a rookie like me it is much simpler if one can use the same
> logging primitives all over or at least the rules when to use what is simple.
> It is simple to say that everything that exists below drivers/gpu/drm/
> relates to drm.
>
> Suggested set of rules to follow:
> - If in drm core, use DRM_XXX where XXX represent the core functionality
> - If in a driver use DRM_DEV* if a struct device is available
> - If in a driver and no struct device, use plain DRM_ERROR/INFO

Core and drivers are already pretty conflated:

http://patchwork.freedesktop.org/patch/msgid/20181227162310.13023-1-jani.nikula@intel.com

---

Side note, I'd like to switch i915 to dev based debugs, but I absolutely
hate the idea of changing:

	DRM_DEBUG_KMS("...")

to:

	DRM_DEV_DEBUG_KMS(dev_priv->drm.dev, "...")

I think the dev based macros are way too long, and would serve *most*
(though not all) drivers better by having struct drm_device * rather
than struct device * as the first param. In the above, just the
boilerplate consumes half the line.

Basically I'd like to see drm_ prefixed analogues to all the dev_ based
logging functions, e.g. drm_dbg that takes drm_device. But it's so much
churn that I'm contemplating just making i915 specific wrappers
instead. :(

BR,
Jani.


>
> If there is a need to distingush before/after one has a drm_device,
> the best way would be to have a set of logging primitives that
> take a drm_device. So we could extend the rule set:
> - If in a driver use DRM_DRM* if a struct drm_device is available
>   (This rule would take precedence over a struct device)
>
> DRM_DRM*, or DRM_DDEV* or ... But you get the idea.
>
> But this is not where we are today.
>
> Shall I redo the patch-set so we go back to dev_*() in probe() / remove()?
>
> 	Sam
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Graphics Center