| Message ID | a67cd754c2f86d7b373b0fa9b0e22f7a217348e2.1568375189.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/print: add and use drm_debug_enabled() | expand |
On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote: > Add helper to check if a drm debug category is enabled. Convert drm core > to use it. No functional changes. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/drm_atomic_uapi.c | 2 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- > drivers/gpu/drm/drm_edid.c | 2 +- > drivers/gpu/drm/drm_edid_load.c | 2 +- > drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- > drivers/gpu/drm/drm_print.c | 4 ++-- > drivers/gpu/drm/drm_vblank.c | 6 +++--- > include/drm/drm_print.h | 5 +++++ > 8 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index 5a5b42db6f2a..6576cd997cbd 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { > ret = drm_atomic_nonblocking_commit(state); > } else { > - if (unlikely(drm_debug & DRM_UT_STATE)) > + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) > drm_atomic_print_state(state); > > ret = drm_atomic_commit(state); > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 97216099a718..f47c5b6b51f7 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, > } > } > out: > - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { > + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { > struct drm_printer p = drm_debug_printer(DBG_PREFIX); > > drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, > idx += tosend + 1; > > ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); > - if (unlikely(ret && drm_debug & DRM_UT_DP)) { > + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { > struct drm_printer p = drm_debug_printer(DBG_PREFIX); > > drm_printf(&p, "sideband msg failed to send\n"); > @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, > mutex_lock(&mgr->qlock); > list_add_tail(&txmsg->next, &mgr->tx_msg_downq); > > - if (unlikely(drm_debug & DRM_UT_DP)) { > + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { > struct drm_printer p = drm_debug_printer(DBG_PREFIX); > > drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 12c783f4d956..58dad4d24cd4 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, > { > int i; > > - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) > + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) > return; > > dev_warn(connector->dev->dev, > diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c > index d38b3b255926..37d8ba3ddb46 100644 > --- a/drivers/gpu/drm/drm_edid_load.c > +++ b/drivers/gpu/drm/drm_edid_load.c > @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, > u8 *edid; > int fwsize, builtin; > int i, valid_extensions = 0; > - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); > + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); > > builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); > if (builtin >= 0) { > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c > index f8154316a3b0..ccfb5b33c5e3 100644 > --- a/drivers/gpu/drm/drm_mipi_dbi.c > +++ b/drivers/gpu/drm/drm_mipi_dbi.c > @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, > int i, ret; > u8 *dst; > > - if (drm_debug & DRM_UT_DRIVER) > + if (drm_debug_enabled(DRM_UT_DRIVER)) > pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > __func__, dc, max_chunk); > > @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, > max_chunk = dbi->tx_buf9_len; > dst16 = dbi->tx_buf9; > > - if (drm_debug & DRM_UT_DRIVER) > + if (drm_debug_enabled(DRM_UT_DRIVER)) > pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > __func__, dc, max_chunk); > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index c9b57012d412..a7c89ec5ff26 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, > struct va_format vaf; > va_list args; > > - if (!(drm_debug & category)) > + if (!drm_debug_enabled(category)) > return; > > va_start(args, format); > @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) > struct va_format vaf; > va_list args; > > - if (!(drm_debug & category)) > + if (!drm_debug_enabled(category)) > return; > > va_start(args, format); > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 9c6899758bc9..4f7962b6427b 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) > u64 vblank; > unsigned long flags; > > - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, > + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, > "This function requires support for accurate vblank timestamps."); > > spin_lock_irqsave(&dev->vblank_time_lock, flags); > @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > */ > *vblank_time = ktime_sub_ns(etime, delta_ns); > > - if ((drm_debug & DRM_UT_VBL) == 0) > + if (!drm_debug_enabled(DRM_UT_VBL)) > return true; > > ts_etime = ktime_to_timespec64(etime); > @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) > assert_spin_locked(&dev->vblank_time_lock); > > vblank = &dev->vblank[pipe]; > - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, > + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, > "Cannot compute missed vblanks without frame duration\n"); > framedur_ns = vblank->framedur_ns; > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index e5c421abce48..e13f901312a4 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) > #define DRM_UT_LEASE 0x80 > #define DRM_UT_DP 0x100 > > +static inline bool drm_debug_enabled(unsigned int category) > +{ > + return drm_debug & category; Worth making that `return unlikely(drm_debug & category);` to make sure the preloaded path is always to skip the debug stuff? > +} > + > __printf(3, 4) > void drm_dev_printk(const struct device *dev, const char *level, > const char *format, ...); > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, 13 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote: >> Add helper to check if a drm debug category is enabled. Convert drm core >> to use it. No functional changes. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/drm_atomic_uapi.c | 2 +- >> drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- >> drivers/gpu/drm/drm_edid.c | 2 +- >> drivers/gpu/drm/drm_edid_load.c | 2 +- >> drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- >> drivers/gpu/drm/drm_print.c | 4 ++-- >> drivers/gpu/drm/drm_vblank.c | 6 +++--- >> include/drm/drm_print.h | 5 +++++ >> 8 files changed, 18 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c >> index 5a5b42db6f2a..6576cd997cbd 100644 >> --- a/drivers/gpu/drm/drm_atomic_uapi.c >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, >> } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { >> ret = drm_atomic_nonblocking_commit(state); >> } else { >> - if (unlikely(drm_debug & DRM_UT_STATE)) >> + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) >> drm_atomic_print_state(state); >> >> ret = drm_atomic_commit(state); >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c >> index 97216099a718..f47c5b6b51f7 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, >> } >> } >> out: >> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { >> + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, >> idx += tosend + 1; >> >> ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); >> - if (unlikely(ret && drm_debug & DRM_UT_DP)) { >> + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> drm_printf(&p, "sideband msg failed to send\n"); >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, >> mutex_lock(&mgr->qlock); >> list_add_tail(&txmsg->next, &mgr->tx_msg_downq); >> >> - if (unlikely(drm_debug & DRM_UT_DP)) { >> + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 12c783f4d956..58dad4d24cd4 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, >> { >> int i; >> >> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) >> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) >> return; >> >> dev_warn(connector->dev->dev, >> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c >> index d38b3b255926..37d8ba3ddb46 100644 >> --- a/drivers/gpu/drm/drm_edid_load.c >> +++ b/drivers/gpu/drm/drm_edid_load.c >> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, >> u8 *edid; >> int fwsize, builtin; >> int i, valid_extensions = 0; >> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); >> + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); >> >> builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); >> if (builtin >= 0) { >> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c >> index f8154316a3b0..ccfb5b33c5e3 100644 >> --- a/drivers/gpu/drm/drm_mipi_dbi.c >> +++ b/drivers/gpu/drm/drm_mipi_dbi.c >> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, >> int i, ret; >> u8 *dst; >> >> - if (drm_debug & DRM_UT_DRIVER) >> + if (drm_debug_enabled(DRM_UT_DRIVER)) >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", >> __func__, dc, max_chunk); >> >> @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, >> max_chunk = dbi->tx_buf9_len; >> dst16 = dbi->tx_buf9; >> >> - if (drm_debug & DRM_UT_DRIVER) >> + if (drm_debug_enabled(DRM_UT_DRIVER)) >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", >> __func__, dc, max_chunk); >> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >> index c9b57012d412..a7c89ec5ff26 100644 >> --- a/drivers/gpu/drm/drm_print.c >> +++ b/drivers/gpu/drm/drm_print.c >> @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, >> struct va_format vaf; >> va_list args; >> >> - if (!(drm_debug & category)) >> + if (!drm_debug_enabled(category)) >> return; >> >> va_start(args, format); >> @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) >> struct va_format vaf; >> va_list args; >> >> - if (!(drm_debug & category)) >> + if (!drm_debug_enabled(category)) >> return; >> >> va_start(args, format); >> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c >> index 9c6899758bc9..4f7962b6427b 100644 >> --- a/drivers/gpu/drm/drm_vblank.c >> +++ b/drivers/gpu/drm/drm_vblank.c >> @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) >> u64 vblank; >> unsigned long flags; >> >> - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, >> "This function requires support for accurate vblank timestamps."); >> >> spin_lock_irqsave(&dev->vblank_time_lock, flags); >> @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >> */ >> *vblank_time = ktime_sub_ns(etime, delta_ns); >> >> - if ((drm_debug & DRM_UT_VBL) == 0) >> + if (!drm_debug_enabled(DRM_UT_VBL)) >> return true; >> >> ts_etime = ktime_to_timespec64(etime); >> @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) >> assert_spin_locked(&dev->vblank_time_lock); >> >> vblank = &dev->vblank[pipe]; >> - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, >> "Cannot compute missed vblanks without frame duration\n"); >> framedur_ns = vblank->framedur_ns; >> >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >> index e5c421abce48..e13f901312a4 100644 >> --- a/include/drm/drm_print.h >> +++ b/include/drm/drm_print.h >> @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) >> #define DRM_UT_LEASE 0x80 >> #define DRM_UT_DP 0x100 >> >> +static inline bool drm_debug_enabled(unsigned int category) >> +{ >> + return drm_debug & category; > > Worth making that `return unlikely(drm_debug & category);` to make sure > the preloaded path is always to skip the debug stuff? If that would let us drop the unlikely() wrapping from the various users of this function, agreed, it would be great. However I'm not at all certain unlikely() here will propagate like that. I just don't know. Do you? BR, Jani. > >> +} >> + >> __printf(3, 4) >> void drm_dev_printk(const struct device *dev, const char *level, >> const char *format, ...); >> -- >> 2.20.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Monday, 2019-09-16 11:53:24 +0300, Jani Nikula wrote: > On Fri, 13 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: > > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote: > >> Add helper to check if a drm debug category is enabled. Convert drm core > >> to use it. No functional changes. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/drm_atomic_uapi.c | 2 +- > >> drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- > >> drivers/gpu/drm/drm_edid.c | 2 +- > >> drivers/gpu/drm/drm_edid_load.c | 2 +- > >> drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- > >> drivers/gpu/drm/drm_print.c | 4 ++-- > >> drivers/gpu/drm/drm_vblank.c | 6 +++--- > >> include/drm/drm_print.h | 5 +++++ > >> 8 files changed, 18 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > >> index 5a5b42db6f2a..6576cd997cbd 100644 > >> --- a/drivers/gpu/drm/drm_atomic_uapi.c > >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c > >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > >> } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { > >> ret = drm_atomic_nonblocking_commit(state); > >> } else { > >> - if (unlikely(drm_debug & DRM_UT_STATE)) > >> + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) > >> drm_atomic_print_state(state); > >> > >> ret = drm_atomic_commit(state); > >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > >> index 97216099a718..f47c5b6b51f7 100644 > >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c > >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, > >> } > >> } > >> out: > >> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { > >> + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { > >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> > >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, > >> idx += tosend + 1; > >> > >> ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); > >> - if (unlikely(ret && drm_debug & DRM_UT_DP)) { > >> + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { > >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> > >> drm_printf(&p, "sideband msg failed to send\n"); > >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, > >> mutex_lock(&mgr->qlock); > >> list_add_tail(&txmsg->next, &mgr->tx_msg_downq); > >> > >> - if (unlikely(drm_debug & DRM_UT_DP)) { > >> + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { > >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> > >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index 12c783f4d956..58dad4d24cd4 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, > >> { > >> int i; > >> > >> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) > >> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) > >> return; > >> > >> dev_warn(connector->dev->dev, > >> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c > >> index d38b3b255926..37d8ba3ddb46 100644 > >> --- a/drivers/gpu/drm/drm_edid_load.c > >> +++ b/drivers/gpu/drm/drm_edid_load.c > >> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, > >> u8 *edid; > >> int fwsize, builtin; > >> int i, valid_extensions = 0; > >> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); > >> + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); > >> > >> builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); > >> if (builtin >= 0) { > >> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c > >> index f8154316a3b0..ccfb5b33c5e3 100644 > >> --- a/drivers/gpu/drm/drm_mipi_dbi.c > >> +++ b/drivers/gpu/drm/drm_mipi_dbi.c > >> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, > >> int i, ret; > >> u8 *dst; > >> > >> - if (drm_debug & DRM_UT_DRIVER) > >> + if (drm_debug_enabled(DRM_UT_DRIVER)) > >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > >> __func__, dc, max_chunk); > >> > >> @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, > >> max_chunk = dbi->tx_buf9_len; > >> dst16 = dbi->tx_buf9; > >> > >> - if (drm_debug & DRM_UT_DRIVER) > >> + if (drm_debug_enabled(DRM_UT_DRIVER)) > >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > >> __func__, dc, max_chunk); > >> > >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > >> index c9b57012d412..a7c89ec5ff26 100644 > >> --- a/drivers/gpu/drm/drm_print.c > >> +++ b/drivers/gpu/drm/drm_print.c > >> @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, > >> struct va_format vaf; > >> va_list args; > >> > >> - if (!(drm_debug & category)) > >> + if (!drm_debug_enabled(category)) > >> return; > >> > >> va_start(args, format); > >> @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) > >> struct va_format vaf; > >> va_list args; > >> > >> - if (!(drm_debug & category)) > >> + if (!drm_debug_enabled(category)) > >> return; > >> > >> va_start(args, format); > >> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > >> index 9c6899758bc9..4f7962b6427b 100644 > >> --- a/drivers/gpu/drm/drm_vblank.c > >> +++ b/drivers/gpu/drm/drm_vblank.c > >> @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) > >> u64 vblank; > >> unsigned long flags; > >> > >> - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, > >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, > >> "This function requires support for accurate vblank timestamps."); > >> > >> spin_lock_irqsave(&dev->vblank_time_lock, flags); > >> @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > >> */ > >> *vblank_time = ktime_sub_ns(etime, delta_ns); > >> > >> - if ((drm_debug & DRM_UT_VBL) == 0) > >> + if (!drm_debug_enabled(DRM_UT_VBL)) > >> return true; > >> > >> ts_etime = ktime_to_timespec64(etime); > >> @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) > >> assert_spin_locked(&dev->vblank_time_lock); > >> > >> vblank = &dev->vblank[pipe]; > >> - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, > >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, > >> "Cannot compute missed vblanks without frame duration\n"); > >> framedur_ns = vblank->framedur_ns; > >> > >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > >> index e5c421abce48..e13f901312a4 100644 > >> --- a/include/drm/drm_print.h > >> +++ b/include/drm/drm_print.h > >> @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) > >> #define DRM_UT_LEASE 0x80 > >> #define DRM_UT_DP 0x100 > >> > >> +static inline bool drm_debug_enabled(unsigned int category) > >> +{ > >> + return drm_debug & category; > > > > Worth making that `return unlikely(drm_debug & category);` to make sure > > the preloaded path is always to skip the debug stuff? > > If that would let us drop the unlikely() wrapping from the various users > of this function, agreed, it would be great. > > However I'm not at all certain unlikely() here will propagate like > that. I just don't know. Do you? I don't know either, but I expect it would? Otherwise, this definitely would: #define drm_debug_enabled(cat) unlikely(drm_debug & category) And yeah, dropping the unlikely() sprinkled around some drm_debug_enabled() calls is a definite bonus. > > BR, > Jani. > > > > > >> +} > >> + > >> __printf(3, 4) > >> void drm_dev_printk(const struct device *dev, const char *level, > >> const char *format, ...); > >> -- > >> 2.20.1 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, 16 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: > On Monday, 2019-09-16 11:53:24 +0300, Jani Nikula wrote: >> On Fri, 13 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: >> > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote: >> >> Add helper to check if a drm debug category is enabled. Convert drm core >> >> to use it. No functional changes. >> >> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> --- >> >> drivers/gpu/drm/drm_atomic_uapi.c | 2 +- >> >> drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- >> >> drivers/gpu/drm/drm_edid.c | 2 +- >> >> drivers/gpu/drm/drm_edid_load.c | 2 +- >> >> drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- >> >> drivers/gpu/drm/drm_print.c | 4 ++-- >> >> drivers/gpu/drm/drm_vblank.c | 6 +++--- >> >> include/drm/drm_print.h | 5 +++++ >> >> 8 files changed, 18 insertions(+), 13 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c >> >> index 5a5b42db6f2a..6576cd997cbd 100644 >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c >> >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, >> >> } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { >> >> ret = drm_atomic_nonblocking_commit(state); >> >> } else { >> >> - if (unlikely(drm_debug & DRM_UT_STATE)) >> >> + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) >> >> drm_atomic_print_state(state); >> >> >> >> ret = drm_atomic_commit(state); >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c >> >> index 97216099a718..f47c5b6b51f7 100644 >> >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, >> >> } >> >> } >> >> out: >> >> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { >> >> + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); >> >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, >> >> idx += tosend + 1; >> >> >> >> ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); >> >> - if (unlikely(ret && drm_debug & DRM_UT_DP)) { >> >> + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> >> >> drm_printf(&p, "sideband msg failed to send\n"); >> >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, >> >> mutex_lock(&mgr->qlock); >> >> list_add_tail(&txmsg->next, &mgr->tx_msg_downq); >> >> >> >> - if (unlikely(drm_debug & DRM_UT_DP)) { >> >> + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); >> >> >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> >> index 12c783f4d956..58dad4d24cd4 100644 >> >> --- a/drivers/gpu/drm/drm_edid.c >> >> +++ b/drivers/gpu/drm/drm_edid.c >> >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, >> >> { >> >> int i; >> >> >> >> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) >> >> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) >> >> return; >> >> >> >> dev_warn(connector->dev->dev, >> >> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c >> >> index d38b3b255926..37d8ba3ddb46 100644 >> >> --- a/drivers/gpu/drm/drm_edid_load.c >> >> +++ b/drivers/gpu/drm/drm_edid_load.c >> >> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, >> >> u8 *edid; >> >> int fwsize, builtin; >> >> int i, valid_extensions = 0; >> >> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); >> >> + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); >> >> >> >> builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); >> >> if (builtin >= 0) { >> >> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c >> >> index f8154316a3b0..ccfb5b33c5e3 100644 >> >> --- a/drivers/gpu/drm/drm_mipi_dbi.c >> >> +++ b/drivers/gpu/drm/drm_mipi_dbi.c >> >> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, >> >> int i, ret; >> >> u8 *dst; >> >> >> >> - if (drm_debug & DRM_UT_DRIVER) >> >> + if (drm_debug_enabled(DRM_UT_DRIVER)) >> >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", >> >> __func__, dc, max_chunk); >> >> >> >> @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, >> >> max_chunk = dbi->tx_buf9_len; >> >> dst16 = dbi->tx_buf9; >> >> >> >> - if (drm_debug & DRM_UT_DRIVER) >> >> + if (drm_debug_enabled(DRM_UT_DRIVER)) >> >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", >> >> __func__, dc, max_chunk); >> >> >> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >> >> index c9b57012d412..a7c89ec5ff26 100644 >> >> --- a/drivers/gpu/drm/drm_print.c >> >> +++ b/drivers/gpu/drm/drm_print.c >> >> @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, >> >> struct va_format vaf; >> >> va_list args; >> >> >> >> - if (!(drm_debug & category)) >> >> + if (!drm_debug_enabled(category)) >> >> return; >> >> >> >> va_start(args, format); >> >> @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) >> >> struct va_format vaf; >> >> va_list args; >> >> >> >> - if (!(drm_debug & category)) >> >> + if (!drm_debug_enabled(category)) >> >> return; >> >> >> >> va_start(args, format); >> >> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c >> >> index 9c6899758bc9..4f7962b6427b 100644 >> >> --- a/drivers/gpu/drm/drm_vblank.c >> >> +++ b/drivers/gpu/drm/drm_vblank.c >> >> @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) >> >> u64 vblank; >> >> unsigned long flags; >> >> >> >> - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, >> >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, >> >> "This function requires support for accurate vblank timestamps."); >> >> >> >> spin_lock_irqsave(&dev->vblank_time_lock, flags); >> >> @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, >> >> */ >> >> *vblank_time = ktime_sub_ns(etime, delta_ns); >> >> >> >> - if ((drm_debug & DRM_UT_VBL) == 0) >> >> + if (!drm_debug_enabled(DRM_UT_VBL)) >> >> return true; >> >> >> >> ts_etime = ktime_to_timespec64(etime); >> >> @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) >> >> assert_spin_locked(&dev->vblank_time_lock); >> >> >> >> vblank = &dev->vblank[pipe]; >> >> - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, >> >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, >> >> "Cannot compute missed vblanks without frame duration\n"); >> >> framedur_ns = vblank->framedur_ns; >> >> >> >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >> >> index e5c421abce48..e13f901312a4 100644 >> >> --- a/include/drm/drm_print.h >> >> +++ b/include/drm/drm_print.h >> >> @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) >> >> #define DRM_UT_LEASE 0x80 >> >> #define DRM_UT_DP 0x100 >> >> >> >> +static inline bool drm_debug_enabled(unsigned int category) >> >> +{ >> >> + return drm_debug & category; >> > >> > Worth making that `return unlikely(drm_debug & category);` to make sure >> > the preloaded path is always to skip the debug stuff? >> >> If that would let us drop the unlikely() wrapping from the various users >> of this function, agreed, it would be great. >> >> However I'm not at all certain unlikely() here will propagate like >> that. I just don't know. Do you? > > I don't know either, but I expect it would? > Otherwise, this definitely would: > #define drm_debug_enabled(cat) unlikely(drm_debug & category) > > And yeah, dropping the unlikely() sprinkled around some drm_debug_enabled() > calls is a definite bonus. With either of the approaches, would it be okay to change call sites like this: - if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { + if (ret && drm_debug_enabled(DRM_UT_DP)) { In theory the compiler should be able to conclude that the unlikely covers the whole branch even if ret is short-circuit evaluated. BR, Jani. > >> >> BR, >> Jani. >> >> >> > >> >> +} >> >> + >> >> __printf(3, 4) >> >> void drm_dev_printk(const struct device *dev, const char *level, >> >> const char *format, ...); >> >> -- >> >> 2.20.1 >> >> >> >> _______________________________________________ >> >> dri-devel mailing list >> >> dri-devel@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > _______________________________________________ >> > dri-devel mailing list >> > dri-devel@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On Monday, 2019-09-16 16:23:13 +0300, Jani Nikula wrote: > On Mon, 16 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: > > On Monday, 2019-09-16 11:53:24 +0300, Jani Nikula wrote: > >> On Fri, 13 Sep 2019, Eric Engestrom <eric.engestrom@intel.com> wrote: > >> > On Friday, 2019-09-13 14:51:39 +0300, Jani Nikula wrote: > >> >> Add helper to check if a drm debug category is enabled. Convert drm core > >> >> to use it. No functional changes. > >> >> > >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> >> --- > >> >> drivers/gpu/drm/drm_atomic_uapi.c | 2 +- > >> >> drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- > >> >> drivers/gpu/drm/drm_edid.c | 2 +- > >> >> drivers/gpu/drm/drm_edid_load.c | 2 +- > >> >> drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- > >> >> drivers/gpu/drm/drm_print.c | 4 ++-- > >> >> drivers/gpu/drm/drm_vblank.c | 6 +++--- > >> >> include/drm/drm_print.h | 5 +++++ > >> >> 8 files changed, 18 insertions(+), 13 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > >> >> index 5a5b42db6f2a..6576cd997cbd 100644 > >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c > >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c > >> >> @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > >> >> } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { > >> >> ret = drm_atomic_nonblocking_commit(state); > >> >> } else { > >> >> - if (unlikely(drm_debug & DRM_UT_STATE)) > >> >> + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) > >> >> drm_atomic_print_state(state); > >> >> > >> >> ret = drm_atomic_commit(state); > >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > >> >> index 97216099a718..f47c5b6b51f7 100644 > >> >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c > >> >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > >> >> @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, > >> >> } > >> >> } > >> >> out: > >> >> - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { > >> >> + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { > >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> >> > >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > >> >> @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, > >> >> idx += tosend + 1; > >> >> > >> >> ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); > >> >> - if (unlikely(ret && drm_debug & DRM_UT_DP)) { > >> >> + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { > >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> >> > >> >> drm_printf(&p, "sideband msg failed to send\n"); > >> >> @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, > >> >> mutex_lock(&mgr->qlock); > >> >> list_add_tail(&txmsg->next, &mgr->tx_msg_downq); > >> >> > >> >> - if (unlikely(drm_debug & DRM_UT_DP)) { > >> >> + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { > >> >> struct drm_printer p = drm_debug_printer(DBG_PREFIX); > >> >> > >> >> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); > >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> >> index 12c783f4d956..58dad4d24cd4 100644 > >> >> --- a/drivers/gpu/drm/drm_edid.c > >> >> +++ b/drivers/gpu/drm/drm_edid.c > >> >> @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, > >> >> { > >> >> int i; > >> >> > >> >> - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) > >> >> + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) > >> >> return; > >> >> > >> >> dev_warn(connector->dev->dev, > >> >> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c > >> >> index d38b3b255926..37d8ba3ddb46 100644 > >> >> --- a/drivers/gpu/drm/drm_edid_load.c > >> >> +++ b/drivers/gpu/drm/drm_edid_load.c > >> >> @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, > >> >> u8 *edid; > >> >> int fwsize, builtin; > >> >> int i, valid_extensions = 0; > >> >> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); > >> >> + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); > >> >> > >> >> builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); > >> >> if (builtin >= 0) { > >> >> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c > >> >> index f8154316a3b0..ccfb5b33c5e3 100644 > >> >> --- a/drivers/gpu/drm/drm_mipi_dbi.c > >> >> +++ b/drivers/gpu/drm/drm_mipi_dbi.c > >> >> @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, > >> >> int i, ret; > >> >> u8 *dst; > >> >> > >> >> - if (drm_debug & DRM_UT_DRIVER) > >> >> + if (drm_debug_enabled(DRM_UT_DRIVER)) > >> >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > >> >> __func__, dc, max_chunk); > >> >> > >> >> @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, > >> >> max_chunk = dbi->tx_buf9_len; > >> >> dst16 = dbi->tx_buf9; > >> >> > >> >> - if (drm_debug & DRM_UT_DRIVER) > >> >> + if (drm_debug_enabled(DRM_UT_DRIVER)) > >> >> pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", > >> >> __func__, dc, max_chunk); > >> >> > >> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > >> >> index c9b57012d412..a7c89ec5ff26 100644 > >> >> --- a/drivers/gpu/drm/drm_print.c > >> >> +++ b/drivers/gpu/drm/drm_print.c > >> >> @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, > >> >> struct va_format vaf; > >> >> va_list args; > >> >> > >> >> - if (!(drm_debug & category)) > >> >> + if (!drm_debug_enabled(category)) > >> >> return; > >> >> > >> >> va_start(args, format); > >> >> @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) > >> >> struct va_format vaf; > >> >> va_list args; > >> >> > >> >> - if (!(drm_debug & category)) > >> >> + if (!drm_debug_enabled(category)) > >> >> return; > >> >> > >> >> va_start(args, format); > >> >> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > >> >> index 9c6899758bc9..4f7962b6427b 100644 > >> >> --- a/drivers/gpu/drm/drm_vblank.c > >> >> +++ b/drivers/gpu/drm/drm_vblank.c > >> >> @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) > >> >> u64 vblank; > >> >> unsigned long flags; > >> >> > >> >> - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, > >> >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, > >> >> "This function requires support for accurate vblank timestamps."); > >> >> > >> >> spin_lock_irqsave(&dev->vblank_time_lock, flags); > >> >> @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > >> >> */ > >> >> *vblank_time = ktime_sub_ns(etime, delta_ns); > >> >> > >> >> - if ((drm_debug & DRM_UT_VBL) == 0) > >> >> + if (!drm_debug_enabled(DRM_UT_VBL)) > >> >> return true; > >> >> > >> >> ts_etime = ktime_to_timespec64(etime); > >> >> @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) > >> >> assert_spin_locked(&dev->vblank_time_lock); > >> >> > >> >> vblank = &dev->vblank[pipe]; > >> >> - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, > >> >> + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, > >> >> "Cannot compute missed vblanks without frame duration\n"); > >> >> framedur_ns = vblank->framedur_ns; > >> >> > >> >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > >> >> index e5c421abce48..e13f901312a4 100644 > >> >> --- a/include/drm/drm_print.h > >> >> +++ b/include/drm/drm_print.h > >> >> @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) > >> >> #define DRM_UT_LEASE 0x80 > >> >> #define DRM_UT_DP 0x100 > >> >> > >> >> +static inline bool drm_debug_enabled(unsigned int category) > >> >> +{ > >> >> + return drm_debug & category; > >> > > >> > Worth making that `return unlikely(drm_debug & category);` to make sure > >> > the preloaded path is always to skip the debug stuff? > >> > >> If that would let us drop the unlikely() wrapping from the various users > >> of this function, agreed, it would be great. > >> > >> However I'm not at all certain unlikely() here will propagate like > >> that. I just don't know. Do you? > > > > I don't know either, but I expect it would? > > Otherwise, this definitely would: > > #define drm_debug_enabled(cat) unlikely(drm_debug & category) > > > > And yeah, dropping the unlikely() sprinkled around some drm_debug_enabled() > > calls is a definite bonus. > > With either of the approaches, would it be okay to change call sites > like this: > > - if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { > + if (ret && drm_debug_enabled(DRM_UT_DP)) { > > In theory the compiler should be able to conclude that the unlikely > covers the whole branch even if ret is short-circuit evaluated. Yeah, the compiler should definitely be smart enough to figure out that `ret && probably_false` is `probably_false` :) > > > BR, > Jani. > > > > > > >> > >> BR, > >> Jani. > >> > >> > >> > > >> >> +} > >> >> + > >> >> __printf(3, 4) > >> >> void drm_dev_printk(const struct device *dev, const char *level, > >> >> const char *format, ...); > >> >> -- > >> >> 2.20.1 > >> >> > >> >> _______________________________________________ > >> >> dri-devel mailing list > >> >> dri-devel@lists.freedesktop.org > >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > _______________________________________________ > >> > dri-devel mailing list > >> > dri-devel@lists.freedesktop.org > >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 5a5b42db6f2a..6576cd997cbd 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1406,7 +1406,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { ret = drm_atomic_nonblocking_commit(state); } else { - if (unlikely(drm_debug & DRM_UT_STATE)) + if (unlikely(drm_debug_enabled(DRM_UT_STATE))) drm_atomic_print_state(state); ret = drm_atomic_commit(state); diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 97216099a718..f47c5b6b51f7 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1180,7 +1180,7 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb, } } out: - if (unlikely(ret == -EIO && drm_debug & DRM_UT_DP)) { + if (unlikely(ret == -EIO && drm_debug_enabled(DRM_UT_DP))) { struct drm_printer p = drm_debug_printer(DBG_PREFIX); drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); @@ -2321,7 +2321,7 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr, idx += tosend + 1; ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx); - if (unlikely(ret && drm_debug & DRM_UT_DP)) { + if (unlikely(ret && drm_debug_enabled(DRM_UT_DP))) { struct drm_printer p = drm_debug_printer(DBG_PREFIX); drm_printf(&p, "sideband msg failed to send\n"); @@ -2388,7 +2388,7 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr, mutex_lock(&mgr->qlock); list_add_tail(&txmsg->next, &mgr->tx_msg_downq); - if (unlikely(drm_debug & DRM_UT_DP)) { + if (unlikely(drm_debug_enabled(DRM_UT_DP))) { struct drm_printer p = drm_debug_printer(DBG_PREFIX); drm_dp_mst_dump_sideband_msg_tx(&p, txmsg); diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12c783f4d956..58dad4d24cd4 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1551,7 +1551,7 @@ static void connector_bad_edid(struct drm_connector *connector, { int i; - if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) + if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) return; dev_warn(connector->dev->dev, diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index d38b3b255926..37d8ba3ddb46 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -175,7 +175,7 @@ static void *edid_load(struct drm_connector *connector, const char *name, u8 *edid; int fwsize, builtin; int i, valid_extensions = 0; - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); + bool print_bad_edid = !connector->bad_edid_counter || drm_debug_enabled(DRM_UT_KMS); builtin = match_string(generic_edid_name, GENERIC_EDIDS, name); if (builtin >= 0) { diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c index f8154316a3b0..ccfb5b33c5e3 100644 --- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -783,7 +783,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc, int i, ret; u8 *dst; - if (drm_debug & DRM_UT_DRIVER) + if (drm_debug_enabled(DRM_UT_DRIVER)) pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", __func__, dc, max_chunk); @@ -907,7 +907,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc, max_chunk = dbi->tx_buf9_len; dst16 = dbi->tx_buf9; - if (drm_debug & DRM_UT_DRIVER) + if (drm_debug_enabled(DRM_UT_DRIVER)) pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n", __func__, dc, max_chunk); diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index c9b57012d412..a7c89ec5ff26 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -264,7 +264,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, struct va_format vaf; va_list args; - if (!(drm_debug & category)) + if (!drm_debug_enabled(category)) return; va_start(args, format); @@ -287,7 +287,7 @@ void drm_dbg(unsigned int category, const char *format, ...) struct va_format vaf; va_list args; - if (!(drm_debug & category)) + if (!drm_debug_enabled(category)) return; va_start(args, format); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 9c6899758bc9..4f7962b6427b 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -332,7 +332,7 @@ u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) u64 vblank; unsigned long flags; - WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !dev->driver->get_vblank_timestamp, "This function requires support for accurate vblank timestamps."); spin_lock_irqsave(&dev->vblank_time_lock, flags); @@ -706,7 +706,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, */ *vblank_time = ktime_sub_ns(etime, delta_ns); - if ((drm_debug & DRM_UT_VBL) == 0) + if (!drm_debug_enabled(DRM_UT_VBL)) return true; ts_etime = ktime_to_timespec64(etime); @@ -1352,7 +1352,7 @@ void drm_vblank_restore(struct drm_device *dev, unsigned int pipe) assert_spin_locked(&dev->vblank_time_lock); vblank = &dev->vblank[pipe]; - WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns, + WARN_ONCE(drm_debug_enabled(DRM_UT_VBL) && !vblank->framedur_ns, "Cannot compute missed vblanks without frame duration\n"); framedur_ns = vblank->framedur_ns; diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index e5c421abce48..e13f901312a4 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -294,6 +294,11 @@ static inline struct drm_printer drm_err_printer(const char *prefix) #define DRM_UT_LEASE 0x80 #define DRM_UT_DP 0x100 +static inline bool drm_debug_enabled(unsigned int category) +{ + return drm_debug & category; +} + __printf(3, 4) void drm_dev_printk(const struct device *dev, const char *level, const char *format, ...);
Add helper to check if a drm debug category is enabled. Convert drm core to use it. No functional changes. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_atomic_uapi.c | 2 +- drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- drivers/gpu/drm/drm_edid.c | 2 +- drivers/gpu/drm/drm_edid_load.c | 2 +- drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- drivers/gpu/drm/drm_print.c | 4 ++-- drivers/gpu/drm/drm_vblank.c | 6 +++--- include/drm/drm_print.h | 5 +++++ 8 files changed, 18 insertions(+), 13 deletions(-)