On 7/10/2024 12:40 AM, Dmitry Baryshkov wrote:
On Tue, 9 Jul 2024 at 22:39, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
On 7/9/2024 6:48 AM, Dmitry Baryshkov wrote:
DPU debugging macros need to be converted to a proper drm_debug_*
macros, however this is a going an intrusive patch, not suitable for a
fix. Wire DPU_DEBUG and DPU_DEBUG_DRIVER to always use DRM_DEBUG_DRIVER
to make sure that DPU debugging messages always end up in the drm debug
messages and are controlled via the usual drm.debug mask.
These macros have been deprecated, is this waht you meant by the
conversion to proper drm_debug_*?
Yes. Drop the driver-specific wrappers where they don't make sense.
Use sensible format strings in the cases where it actually does (like
VIDENC or _PLANE)
Ack but we need to not just drop the wrappers but drop the usage of
these macros as well because it is documented that they are deprecated.
So I assume you want to get this in and do that as a follow up change?
/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
#define DRM_DEBUG_DRIVER(fmt, ...) \
__drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
I think all that this macro was doing was to have appropriate DRM_UT_*
macros enabled before calling the corresponding DRM_DEBUG_* macros. But
I think what was incorrect here is for DPU_DEBUG, we could have used
DRM_UT_CORE instead of DRM_UT_KMS.
It pretty much tries to overplay the existing drm debugging mechanism
by either sending the messages to the DRM channel or just using
pr_debug. With DYNAMIC_DEBUG being disabled pr_debug is just an empty
macro, so all the messages can end up in /dev/null. We should not be
trying to be too smart, using standard DRM_DEBUG_DRIVER should be
enough. This way all driver-related messages are controlled by
drm.debug including or excluding the 0x02 bit.
And DRM_DEBUG_DRIVER should have been used instead of DRM_ERROR.
Was this causing the issue of the prints not getting enabled?
I pretty much think so.
Alright, I am okay with the approach, just one minor suggestion, to keep
the behavior intact, previously the code wanted DPU_DEBUG to be
controlled by DRM_UT_KMS and DPU_DEBUG_DRIVER controlled by DRM_UT_DRIVER.
Keeping that intact, we need to use DRM_DEBUG_KMS for DPU_DEBUG?
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index e2adc937ea63..935ff6fd172c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -31,24 +31,14 @@
* @fmt: Pointer to format string
*/
#define DPU_DEBUG(fmt, ...) \
- do { \
- if (drm_debug_enabled(DRM_UT_KMS)) \
- DRM_DEBUG(fmt, ##__VA_ARGS__); \
- else \
- pr_debug(fmt, ##__VA_ARGS__); \
- } while (0)
+ DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
/**
* DPU_DEBUG_DRIVER - macro for hardware driver logging
* @fmt: Pointer to format string
*/
#define DPU_DEBUG_DRIVER(fmt, ...) \
- do { \
- if (drm_debug_enabled(DRM_UT_DRIVER)) \
- DRM_ERROR(fmt, ##__VA_ARGS__); \
- else \
- pr_debug(fmt, ##__VA_ARGS__); \
- } while (0)
+ DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
#define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__)
