Re: [PATCH v7 3/3] drm: Switch to %p4cc format modifier

From
Sakari Ailus <sakari.ailus@linux.intel.com>
Date
2021-02-15 13:52:31
Also in
dri-devel, lkml
Reply to
Andy Shevchenko, 2021-02-15
Hi Andy,

On Mon, Feb 15, 2021 at 03:41:14PM +0200, Andy Shevchenko wrote:
On Mon, Feb 15, 2021 at 01:40:30PM +0200, Sakari Ailus wrote:
quoted
Switch DRM drivers from drm_get_format_name() to %p4cc. This gets rid of a
large number of temporary variables at the same time.
What a nice clean up!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks!
after addressing nit-picks below.

Side note (no need to implement esp. right now): it seems often it's coupled
with modifier, would be nice to have them together that %p4ccM or so can do it
in one go.
...
quoted
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 03262472059c..5cf45aa6eedc 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -30,11 +30,6 @@
 #include <drm/drm_device.h>
 #include <drm/drm_fourcc.h>
 
-static char printable_char(int c)
-{
-	return isascii(c) && isprint(c) ? c : '?';
-}
If it goes as an ABI than your dot is incompatible with this and '?' should be
used instead in the patch 1. And I bend towards suggested '?' rather than '.'.

Also it means that you probably would need different specifiers for full and
short formats.
I thought of that, but then the resulting string would be indeed different
and comparing short and long formats would be harder. Remember this is for
debug prints. If the format is too long, then it should be made shorter,
but during the earlier review rounds people have expressed interest in
having this information there.

"?" can be expanded by the shell whereas "." is not. If DRM folks think we
should go back to "?" I'm fine with that. Also note that there's something
wrong with the fourcc code to begin with if it's got either "." or "?".

-- 
Regards,

Sakari Ailus
Thread (22 messages) 22 messages, 5 authors, last reply 2021-02-16
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help