linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: "Christian König" <christian.koenig@amd.com>,
	"Bernard Zhao" <bernard@vivo.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"David (ChunMing) Zhou" <David1.Zhou@amd.com>,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Cc: opensource.kernel@vivo.com
Subject: Re: [PATCH] drm/radeon: cleanup coding style a bit
Date: Sun, 26 Apr 2020 11:06:27 -0700	[thread overview]
Message-ID: <8b9bf44c9f823b887ca3477f3b6bca383c0eba4e.camel@perches.com> (raw)
In-Reply-To: <2c67e29b-df17-6ae3-cb55-af950acc022c@amd.com>

btw:  the debugging macros in atom.c are not good.

It could be something like the below as the output logging is
at best poorly formatted due to the many individual printks
without KERN_<LEVEL> that are emitted on separate lines.

#define ATOM_DEBUG

should probably be commented out.

The debugging macros and #include file should be better formatted.

The no_printk macro is useful to verify formats and arguments when
not debugging and removing the ATOM_DEBUG from atom-names.h does
not cause the unused char *arrays to be added to the object file
as the compiler elides unused arrays.

---
 drivers/gpu/drm/radeon/atom-names.h | 266 +++++++++++++++++++++++++++---------
 drivers/gpu/drm/radeon/atom.c       |  23 ++--
 2 files changed, 218 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atom-names.h b/drivers/gpu/drm/radeon/atom-names.h
index 6f907a5..055775 100644
--- a/drivers/gpu/drm/radeon/atom-names.h
+++ b/drivers/gpu/drm/radeon/atom-names.h
@@ -27,74 +27,218 @@
 
 #include "atom.h"
 
-#ifdef ATOM_DEBUG
-
 #define ATOM_OP_NAMES_CNT 123
-static char *atom_op_names[ATOM_OP_NAMES_CNT] = {
-"RESERVED", "MOVE_REG", "MOVE_PS", "MOVE_WS", "MOVE_FB", "MOVE_PLL",
-"MOVE_MC", "AND_REG", "AND_PS", "AND_WS", "AND_FB", "AND_PLL", "AND_MC",
-"OR_REG", "OR_PS", "OR_WS", "OR_FB", "OR_PLL", "OR_MC", "SHIFT_LEFT_REG",
-"SHIFT_LEFT_PS", "SHIFT_LEFT_WS", "SHIFT_LEFT_FB", "SHIFT_LEFT_PLL",
-"SHIFT_LEFT_MC", "SHIFT_RIGHT_REG", "SHIFT_RIGHT_PS", "SHIFT_RIGHT_WS",
-"SHIFT_RIGHT_FB", "SHIFT_RIGHT_PLL", "SHIFT_RIGHT_MC", "MUL_REG",
-"MUL_PS", "MUL_WS", "MUL_FB", "MUL_PLL", "MUL_MC", "DIV_REG", "DIV_PS",
-"DIV_WS", "DIV_FB", "DIV_PLL", "DIV_MC", "ADD_REG", "ADD_PS", "ADD_WS",
-"ADD_FB", "ADD_PLL", "ADD_MC", "SUB_REG", "SUB_PS", "SUB_WS", "SUB_FB",
-"SUB_PLL", "SUB_MC", "SET_ATI_PORT", "SET_PCI_PORT", "SET_SYS_IO_PORT",
-"SET_REG_BLOCK", "SET_FB_BASE", "COMPARE_REG", "COMPARE_PS",
-"COMPARE_WS", "COMPARE_FB", "COMPARE_PLL", "COMPARE_MC", "SWITCH",
-"JUMP", "JUMP_EQUAL", "JUMP_BELOW", "JUMP_ABOVE", "JUMP_BELOW_OR_EQUAL",
-"JUMP_ABOVE_OR_EQUAL", "JUMP_NOT_EQUAL", "TEST_REG", "TEST_PS", "TEST_WS",
-"TEST_FB", "TEST_PLL", "TEST_MC", "DELAY_MILLISEC", "DELAY_MICROSEC",
-"CALL_TABLE", "REPEAT", "CLEAR_REG", "CLEAR_PS", "CLEAR_WS", "CLEAR_FB",
-"CLEAR_PLL", "CLEAR_MC", "NOP", "EOT", "MASK_REG", "MASK_PS", "MASK_WS",
-"MASK_FB", "MASK_PLL", "MASK_MC", "POST_CARD", "BEEP", "SAVE_REG",
-"RESTORE_REG", "SET_DATA_BLOCK", "XOR_REG", "XOR_PS", "XOR_WS", "XOR_FB",
-"XOR_PLL", "XOR_MC", "SHL_REG", "SHL_PS", "SHL_WS", "SHL_FB", "SHL_PLL",
-"SHL_MC", "SHR_REG", "SHR_PS", "SHR_WS", "SHR_FB", "SHR_PLL", "SHR_MC",
-"DEBUG", "CTB_DS",
+static const char * const atom_op_names[ATOM_OP_NAMES_CNT] = {
+	"RESERVED",
+	"MOVE_REG",
+	"MOVE_PS",
+	"MOVE_WS",
+	"MOVE_FB",
+	"MOVE_PLL",
+	"MOVE_MC",
+	"AND_REG",
+	"AND_PS",
+	"AND_WS",
+	"AND_FB",
+	"AND_PLL",
+	"AND_MC",
+	"OR_REG",
+	"OR_PS",
+	"OR_WS",
+	"OR_FB",
+	"OR_PLL",
+	"OR_MC",
+	"SHIFT_LEFT_REG",
+	"SHIFT_LEFT_PS",
+	"SHIFT_LEFT_WS",
+	"SHIFT_LEFT_FB",
+	"SHIFT_LEFT_PLL",
+	"SHIFT_LEFT_MC",
+	"SHIFT_RIGHT_REG",
+	"SHIFT_RIGHT_PS",
+	"SHIFT_RIGHT_WS",
+	"SHIFT_RIGHT_FB",
+	"SHIFT_RIGHT_PLL",
+	"SHIFT_RIGHT_MC",
+	"MUL_REG",
+	"MUL_PS",
+	"MUL_WS",
+	"MUL_FB",
+	"MUL_PLL",
+	"MUL_MC",
+	"DIV_REG",
+	"DIV_PS",
+	"DIV_WS",
+	"DIV_FB",
+	"DIV_PLL",
+	"DIV_MC",
+	"ADD_REG",
+	"ADD_PS",
+	"ADD_WS",
+	"ADD_FB",
+	"ADD_PLL",
+	"ADD_MC",
+	"SUB_REG",
+	"SUB_PS",
+	"SUB_WS",
+	"SUB_FB",
+	"SUB_PLL",
+	"SUB_MC",
+	"SET_ATI_PORT",
+	"SET_PCI_PORT",
+	"SET_SYS_IO_PORT",
+	"SET_REG_BLOCK",
+	"SET_FB_BASE",
+	"COMPARE_REG",
+	"COMPARE_PS",
+	"COMPARE_WS",
+	"COMPARE_FB",
+	"COMPARE_PLL",
+	"COMPARE_MC",
+	"SWITCH",
+	"JUMP",
+	"JUMP_EQUAL",
+	"JUMP_BELOW",
+	"JUMP_ABOVE",
+	"JUMP_BELOW_OR_EQUAL",
+	"JUMP_ABOVE_OR_EQUAL",
+	"JUMP_NOT_EQUAL",
+	"TEST_REG",
+	"TEST_PS",
+	"TEST_WS",
+	"TEST_FB",
+	"TEST_PLL",
+	"TEST_MC",
+	"DELAY_MILLISEC",
+	"DELAY_MICROSEC",
+	"CALL_TABLE",
+	"REPEAT",
+	"CLEAR_REG",
+	"CLEAR_PS",
+	"CLEAR_WS",
+	"CLEAR_FB",
+	"CLEAR_PLL",
+	"CLEAR_MC",
+	"NOP",
+	"EOT",
+	"MASK_REG",
+	"MASK_PS",
+	"MASK_WS",
+	"MASK_FB",
+	"MASK_PLL",
+	"MASK_MC",
+	"POST_CARD",
+	"BEEP",
+	"SAVE_REG",
+	"RESTORE_REG",
+	"SET_DATA_BLOCK",
+	"XOR_REG",
+	"XOR_PS",
+	"XOR_WS",
+	"XOR_FB",
+	"XOR_PLL",
+	"XOR_MC",
+	"SHL_REG",
+	"SHL_PS",
+	"SHL_WS",
+	"SHL_FB",
+	"SHL_PLL",
+	"SHL_MC",
+	"SHR_REG",
+	"SHR_PS",
+	"SHR_WS",
+	"SHR_FB",
+	"SHR_PLL",
+	"SHR_MC",
+	"DEBUG",
+	"CTB_DS",
 };
 
 #define ATOM_TABLE_NAMES_CNT 74
-static char *atom_table_names[ATOM_TABLE_NAMES_CNT] = {
-"ASIC_Init", "GetDisplaySurfaceSize", "ASIC_RegistersInit",
-"VRAM_BlockVenderDetection", "SetClocksRatio", "MemoryControllerInit",
-"GPIO_PinInit", "MemoryParamAdjust", "DVOEncoderControl",
-"GPIOPinControl", "SetEngineClock", "SetMemoryClock", "SetPixelClock",
-"DynamicClockGating", "ResetMemoryDLL", "ResetMemoryDevice",
-"MemoryPLLInit", "EnableMemorySelfRefresh", "AdjustMemoryController",
-"EnableASIC_StaticPwrMgt", "ASIC_StaticPwrMgtStatusChange",
-"DAC_LoadDetection", "TMDS2EncoderControl", "LCD1OutputControl",
-"DAC1EncoderControl", "DAC2EncoderControl", "DVOOutputControl",
-"CV1OutputControl", "SetCRTC_DPM_State", "TVEncoderControl",
-"TMDS1EncoderControl", "LVDSEncoderControl", "TV1OutputControl",
-"EnableScaler", "BlankCRTC", "EnableCRTC", "GetPixelClock",
-"EnableVGA_Render", "EnableVGA_Access", "SetCRTC_Timing",
-"SetCRTC_OverScan", "SetCRTC_Replication", "SelectCRTC_Source",
-"EnableGraphSurfaces", "UpdateCRTC_DoubleBufferRegisters",
-"LUT_AutoFill", "EnableHW_IconCursor", "GetMemoryClock",
-"GetEngineClock", "SetCRTC_UsingDTDTiming", "TVBootUpStdPinDetection",
-"DFP2OutputControl", "VRAM_BlockDetectionByStrap", "MemoryCleanUp",
-"ReadEDIDFromHWAssistedI2C", "WriteOneByteToHWAssistedI2C",
-"ReadHWAssistedI2CStatus", "SpeedFanControl", "PowerConnectorDetection",
-"MC_Synchronization", "ComputeMemoryEnginePLL", "MemoryRefreshConversion",
-"VRAM_GetCurrentInfoBlock", "DynamicMemorySettings", "MemoryTraining",
-"EnableLVDS_SS", "DFP1OutputControl", "SetVoltage", "CRT1OutputControl",
-"CRT2OutputControl", "SetupHWAssistedI2CStatus", "ClockSource",
-"MemoryDeviceInit", "EnableYUV",
+static const char * const atom_table_names[ATOM_TABLE_NAMES_CNT] = {
+	"ASIC_Init",
+	"GetDisplaySurfaceSize",
+	"ASIC_RegistersInit",
+	"VRAM_BlockVenderDetection",
+	"SetClocksRatio",
+	"MemoryControllerInit",
+	"GPIO_PinInit",
+	"MemoryParamAdjust",
+	"DVOEncoderControl",
+	"GPIOPinControl",
+	"SetEngineClock",
+	"SetMemoryClock",
+	"SetPixelClock",
+	"DynamicClockGating",
+	"ResetMemoryDLL",
+	"ResetMemoryDevice",
+	"MemoryPLLInit",
+	"EnableMemorySelfRefresh",
+	"AdjustMemoryController",
+	"EnableASIC_StaticPwrMgt",
+	"ASIC_StaticPwrMgtStatusChange",
+	"DAC_LoadDetection",
+	"TMDS2EncoderControl",
+	"LCD1OutputControl",
+	"DAC1EncoderControl",
+	"DAC2EncoderControl",
+	"DVOOutputControl",
+	"CV1OutputControl",
+	"SetCRTC_DPM_State",
+	"TVEncoderControl",
+	"TMDS1EncoderControl",
+	"LVDSEncoderControl",
+	"TV1OutputControl",
+	"EnableScaler",
+	"BlankCRTC",
+	"EnableCRTC",
+	"GetPixelClock",
+	"EnableVGA_Render",
+	"EnableVGA_Access",
+	"SetCRTC_Timing",
+	"SetCRTC_OverScan",
+	"SetCRTC_Replication",
+	"SelectCRTC_Source",
+	"EnableGraphSurfaces",
+	"UpdateCRTC_DoubleBufferRegisters",
+	"LUT_AutoFill",
+	"EnableHW_IconCursor",
+	"GetMemoryClock",
+	"GetEngineClock",
+	"SetCRTC_UsingDTDTiming",
+	"TVBootUpStdPinDetection",
+	"DFP2OutputControl",
+	"VRAM_BlockDetectionByStrap",
+	"MemoryCleanUp",
+	"ReadEDIDFromHWAssistedI2C",
+	"WriteOneByteToHWAssistedI2C",
+	"ReadHWAssistedI2CStatus",
+	"SpeedFanControl",
+	"PowerConnectorDetection",
+	"MC_Synchronization",
+	"ComputeMemoryEnginePLL",
+	"MemoryRefreshConversion",
+	"VRAM_GetCurrentInfoBlock",
+	"DynamicMemorySettings",
+	"MemoryTraining",
+	"EnableLVDS_SS",
+	"DFP1OutputControl",
+	"SetVoltage",
+	"CRT1OutputControl",
+	"CRT2OutputControl",
+	"SetupHWAssistedI2CStatus",
+	"ClockSource",
+	"MemoryDeviceInit",
+	"EnableYUV",
 };
 
 #define ATOM_IO_NAMES_CNT 5
-static char *atom_io_names[ATOM_IO_NAMES_CNT] = {
-"MM", "PLL", "MC", "PCIE", "PCIE PORT",
+static const char * const atom_io_names[ATOM_IO_NAMES_CNT] = {
+	"MM",
+	"PLL",
+	"MC",
+	"PCIE",
+	"PCIE PORT",
 };
 
-#else
-
-#define ATOM_OP_NAMES_CNT 0
-#define ATOM_TABLE_NAMES_CNT 0
-#define ATOM_IO_NAMES_CNT 0
-
-#endif
-
 #endif
diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 2c2762..892eeb 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -91,17 +91,20 @@ static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
 
 static int debug_depth = 0;
 #ifdef ATOM_DEBUG
-static void debug_print_spaces(int n)
-{
-	while (n--)
-		printk("   ");
-}
-
-#define DEBUG(...) do if (atom_debug) { printk(KERN_DEBUG __VA_ARGS__); } while (0)
-#define SDEBUG(...) do if (atom_debug) { printk(KERN_DEBUG); debug_print_spaces(debug_depth); printk(__VA_ARGS__); } while (0)
+#define DEBUG(fmt, ...)							\
+do {									\
+	if (atom_debug)							\
+		printk(KERN_DEBUG fmt, ##__VA_ARGS__);			\
+} while (0)
+#define SDEBUG(fmt, ...)						\
+do {									\
+	if (atom_debug)							\
+		printk(KERN_DEBUG "%*s" fmt,				\
+		       debug_depth * 3, "", ##__VA_ARGS__);		\
+} while (0)
 #else
-#define DEBUG(...) do { } while (0)
-#define SDEBUG(...) do { } while (0)
+#define DEBUG(fmt, ...)		no_printk(fmt, ##__VA_ARGS__)
+#define SDEBUG(fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
 static uint32_t atom_iio_execute(struct atom_context *ctx, int base,





  parent reply	other threads:[~2020-04-26 18:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-26 13:12 [PATCH] drm/radeon: cleanup coding style a bit Bernard Zhao
2020-04-26 13:18 ` Christian König
2020-04-26 17:53   ` Joe Perches
2020-04-30 11:00     ` Bernard
2020-04-30 13:32       ` Christian König
2020-04-26 18:06   ` Joe Perches [this message]
2020-04-30 14:53   ` Alex Deucher

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=8b9bf44c9f823b887ca3477f3b6bca383c0eba4e.camel@perches.com \
    --to=joe@perches.com \
    --cc=David1.Zhou@amd.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=bernard@vivo.com \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=opensource.kernel@vivo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).