On Thu, Jan 31, 2019 at 10:03:12PM +0100, Sam Ravnborg wrote: > Hi Sean. > > > Hey Sam, > > Thanks for the patchset, this will make dmesg grepping easier! One comment, and > > you're going to hate me for it: Why use DRM_DEV* instead of DRM_*? > > > > When I introduced DRM_DEV, it was to cover the case where there are multiple > > instances of the same driver (ie: dual-channel mipi, multiple crtcs, etc). I > > suppose that _could_ happen in the panel space, but it seems more unlikely than > > not. > > The rationale for using DRM_DEV* are solely that if a struct device * is avalible, > then we can use this to provide more information about the origin of the logging. > > I have not testet it - but from browsing the code I could not see that > DRM_ERROR and friends picked up the module name. > If DRM_ERROR is the right choice I will redo the patches - no problem. > > But if we loose the module name then the DRM_DEV* variants are preferable IMO. 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_*(). The reason for this is that during most of ->probe() there's not really any connection to DRM. In many cases the DRM device doesn't even exist yet. Consider component/master setups where some display component will wait for the panel to be registered before binding the master. So I find it confusing to have the DRM style messages when there's actually no DRM going on yet. That said, I understand that this might not be an immediately obvious rule and it might not be followed rigorously, which both quite possibly contribute to the perception that the messages are all inconsistent. It always made sense to me, but if everyone else thinks that it's totally nuts, I'm sure I can find a way to get over it. Thierry
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
