* [PATCH v2 1/9] drm/i915/display/display_power: Prefer drm_WARN_ON over WARN_ON
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 2/9] drm/i915/display/dp: Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Imre Deak, Ville Syrjälä,
José Roberto de Souza, Chris Wilson, Matt Roper,
Anshuman Gupta
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON at places where struct i915_power_domains
struct is available.
Conversion is done with below sementic patch:
@@
identifier func, T;
@@
func(struct i915_power_domains *T,...) {
+ struct drm_i915_private *i915 = container_of(T, struct drm_i915_private, power_domains);
<+...
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
...+>
}
changes since v1:
- Fix commit subject (Jani)
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
.../drm/i915/display/intel_display_power.c | 35 +++++++++++++------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 49998906cc61..69039cea1b5a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1943,22 +1943,29 @@ static u64 __async_put_domains_mask(struct i915_power_domains *power_domains)
static bool
assert_async_put_domain_masks_disjoint(struct i915_power_domains *power_domains)
{
- return !WARN_ON(power_domains->async_put_domains[0] &
- power_domains->async_put_domains[1]);
+ struct drm_i915_private *i915 = container_of(power_domains,
+ struct drm_i915_private,
+ power_domains);
+ return !drm_WARN_ON(&i915->drm, power_domains->async_put_domains[0] &
+ power_domains->async_put_domains[1]);
}
static bool
__async_put_domains_state_ok(struct i915_power_domains *power_domains)
{
+ struct drm_i915_private *i915 = container_of(power_domains,
+ struct drm_i915_private,
+ power_domains);
enum intel_display_power_domain domain;
bool err = false;
err |= !assert_async_put_domain_masks_disjoint(power_domains);
- err |= WARN_ON(!!power_domains->async_put_wakeref !=
- !!__async_put_domains_mask(power_domains));
+ err |= drm_WARN_ON(&i915->drm, !!power_domains->async_put_wakeref !=
+ !!__async_put_domains_mask(power_domains));
for_each_power_domain(domain, __async_put_domains_mask(power_domains))
- err |= WARN_ON(power_domains->domain_use_count[domain] != 1);
+ err |= drm_WARN_ON(&i915->drm,
+ power_domains->domain_use_count[domain] != 1);
return !err;
}
@@ -2200,11 +2207,14 @@ static void
queue_async_put_domains_work(struct i915_power_domains *power_domains,
intel_wakeref_t wakeref)
{
- WARN_ON(power_domains->async_put_wakeref);
+ struct drm_i915_private *i915 = container_of(power_domains,
+ struct drm_i915_private,
+ power_domains);
+ drm_WARN_ON(&i915->drm, power_domains->async_put_wakeref);
power_domains->async_put_wakeref = wakeref;
- WARN_ON(!queue_delayed_work(system_unbound_wq,
- &power_domains->async_put_work,
- msecs_to_jiffies(100)));
+ drm_WARN_ON(&i915->drm, !queue_delayed_work(system_unbound_wq,
+ &power_domains->async_put_work,
+ msecs_to_jiffies(100)));
}
static void
@@ -4365,6 +4375,9 @@ __set_power_wells(struct i915_power_domains *power_domains,
const struct i915_power_well_desc *power_well_descs,
int power_well_count)
{
+ struct drm_i915_private *i915 = container_of(power_domains,
+ struct drm_i915_private,
+ power_domains);
u64 power_well_ids = 0;
int i;
@@ -4384,8 +4397,8 @@ __set_power_wells(struct i915_power_domains *power_domains,
if (id == DISP_PW_ID_NONE)
continue;
- WARN_ON(id >= sizeof(power_well_ids) * 8);
- WARN_ON(power_well_ids & BIT_ULL(id));
+ drm_WARN_ON(&i915->drm, id >= sizeof(power_well_ids) * 8);
+ drm_WARN_ON(&i915->drm, power_well_ids & BIT_ULL(id));
power_well_ids |= BIT_ULL(id);
}
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 2/9] drm/i915/display/dp: Prefer drm_WARN* over WARN*
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 1/9] drm/i915/display/display_power: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 3/9] drm/i915/display/sdvo: " Pankaj Bharadiya
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Ville Syrjälä,
Manasi Navare, José Roberto de Souza, Chris Wilson,
Imre Deak, Gwan-gyeong Mun, Matt Roper
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN* at places where struct intel_dp or struct
drm_i915_private pointer is available.
Conversion is done with below sementic patch:
@rule1@
identifier func, T;
@@
func(...) {
...
struct drm_i915_private *T = ...;
<+...
(
-WARN_ON(
+drm_WARN_ON(&T->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&T->drm,
...)
)
...+>
}
@rule2@
identifier func, T;
@@
func(struct drm_i915_private *T,...) {
<+...
(
-WARN_ON(
+drm_WARN_ON(&T->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&T->drm,
...)
)
...+>
}
@rule3@
identifier func, T;
@@
func(struct intel_dp *T,...) {
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
@rule4@
identifier func, T;
@@
func(...) {
...
struct intel_dp *T = ...;
+ struct drm_i915_private *i915 = dp_to_i915(T);
<+...
(
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6952b0295096..ac44fc242879 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -409,7 +409,10 @@ static int intel_dp_rate_index(const int *rates, int len, int rate)
static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
{
- WARN_ON(!intel_dp->num_source_rates || !intel_dp->num_sink_rates);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+ drm_WARN_ON(&i915->drm,
+ !intel_dp->num_source_rates || !intel_dp->num_sink_rates);
intel_dp->num_common_rates = intersect_rates(intel_dp->source_rates,
intel_dp->num_source_rates,
@@ -418,7 +421,7 @@ static void intel_dp_set_common_rates(struct intel_dp *intel_dp)
intel_dp->common_rates);
/* Paranoia, there should always be something in common. */
- if (WARN_ON(intel_dp->num_common_rates == 0)) {
+ if (drm_WARN_ON(&i915->drm, intel_dp->num_common_rates == 0)) {
intel_dp->common_rates[0] = 162000;
intel_dp->num_common_rates = 1;
}
@@ -1554,6 +1557,7 @@ static ssize_t
intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
{
struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
u8 txbuf[20], rxbuf[20];
size_t txsize, rxsize;
int ret;
@@ -1567,10 +1571,10 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE;
rxsize = 2; /* 0 or 1 data bytes */
- if (WARN_ON(txsize > 20))
+ if (drm_WARN_ON(&i915->drm, txsize > 20))
return -E2BIG;
- WARN_ON(!msg->buffer != !msg->size);
+ drm_WARN_ON(&i915->drm, !msg->buffer != !msg->size);
if (msg->buffer)
memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
@@ -1595,7 +1599,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
rxsize = msg->size + 1;
- if (WARN_ON(rxsize > 20))
+ if (drm_WARN_ON(&i915->drm, rxsize > 20))
return -E2BIG;
ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
@@ -1870,10 +1874,11 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
int
intel_dp_max_link_rate(struct intel_dp *intel_dp)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int len;
len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
- if (WARN_ON(len <= 0))
+ if (drm_WARN_ON(&i915->drm, len <= 0))
return 162000;
return intel_dp->common_rates[len - 1];
@@ -1881,10 +1886,11 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
int i = intel_dp_rate_index(intel_dp->sink_rates,
intel_dp->num_sink_rates, rate);
- if (WARN_ON(i < 0))
+ if (drm_WARN_ON(&i915->drm, i < 0))
i = 0;
return i;
@@ -5580,7 +5586,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
if (!intel_dp->is_mst)
return -EINVAL;
- WARN_ON_ONCE(intel_dp->active_mst_links < 0);
+ drm_WARN_ON_ONCE(&i915->drm, intel_dp->active_mst_links < 0);
for (;;) {
u8 esi[DP_DPRX_ESI_LEN] = {};
@@ -5942,7 +5948,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
u8 *dpcd = intel_dp->dpcd;
u8 type;
- if (WARN_ON(intel_dp_is_edp(intel_dp)))
+ if (drm_WARN_ON(&i915->drm, intel_dp_is_edp(intel_dp)))
return connector_status_connected;
if (lspcon->active)
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 1/9] drm/i915/display/display_power: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 2/9] drm/i915/display/dp: Prefer drm_WARN* over WARN* Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-08 6:48 ` Jani Nikula
2020-05-04 18:15 ` [PATCH v2 4/9] drm/i915/display/tc: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
` (5 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Ville Syrjälä,
Chris Wilson, Imre Deak, Maarten Lankhorst, Pankaj Bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN* calls.
changes since v1:
- Added dev_priv local variable and used it in drm_WARN* calls (Jani)
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index bc6c26818e15..773523dcd107 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd)
static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
const void *args, int args_len)
{
+ struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
const char *cmd_name;
int i, pos = 0;
char buffer[64];
@@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
else
BUF_PRINT("(%02X)", cmd);
- WARN_ON(pos >= sizeof(buffer) - 1);
+ drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
#undef BUF_PRINT
DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
@@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
void *response, int response_len)
{
+ struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
const char *cmd_status;
u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
u8 status;
@@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
BUF_PRINT(" %02X", ((u8 *)response)[i]);
}
- WARN_ON(pos >= sizeof(buffer) - 1);
+ drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
#undef BUF_PRINT
DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
@@ -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
struct intel_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
+ struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
const struct drm_display_mode *adjusted_mode =
&crtc_state->hw.adjusted_mode;
@@ -1106,7 +1109,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
HDMI_QUANTIZATION_RANGE_FULL);
ret = hdmi_avi_infoframe_check(frame);
- if (WARN_ON(ret))
+ if (drm_WARN_ON(&dev_priv->drm, ret))
return false;
return true;
@@ -1115,6 +1118,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
const struct intel_crtc_state *crtc_state)
{
+ struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
const union hdmi_infoframe *frame = &crtc_state->infoframes.avi;
ssize_t len;
@@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
return true;
- if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
+ if (drm_WARN_ON(&dev_priv->drm,
+ frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
return false;
len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
- if (WARN_ON(len < 0))
+ if (drm_WARN_ON(&dev_priv->drm, len < 0))
return false;
return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
@@ -1237,6 +1242,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
{
+ struct drm_i915_private *dev_priv = to_i915(pipe_config->uapi.crtc->dev);
unsigned dotclock = pipe_config->port_clock;
struct dpll *clock = &pipe_config->dpll;
@@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
clock->m1 = 12;
clock->m2 = 8;
} else {
- WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
+ drm_WARN(&dev_priv->drm, 1,
+ "SDVO TV clock out of range: %i\n", dotclock);
}
pipe_config->clock_set = true;
@@ -2293,7 +2300,7 @@ intel_sdvo_connector_atomic_get_property(struct drm_connector *connector,
return 0;
}
- WARN_ON(1);
+ drm_WARN_ON(connector->dev, 1);
*val = 0;
} else if (property == intel_sdvo_connector->top ||
property == intel_sdvo_connector->bottom)
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
2020-05-04 18:15 ` [PATCH v2 3/9] drm/i915/display/sdvo: " Pankaj Bharadiya
@ 2020-05-08 6:48 ` Jani Nikula
2020-05-08 7:23 ` Laxminarayan Bharadiya, Pankaj
0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-05-08 6:48 UTC (permalink / raw)
To: Pankaj Bharadiya, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Ville Syrjälä,
Chris Wilson, Imre Deak, Maarten Lankhorst, Pankaj Bharadiya
On Mon, 04 May 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> struct drm_device specific drm_WARN* macros include device information
> in the backtrace, so we know what device the warnings originate from.
>
> Prefer drm_WARN* over WARN* calls.
>
> changes since v1:
> - Added dev_priv local variable and used it in drm_WARN* calls (Jani)
In the earlier patches you're adding i915 local variable, here it's
dev_priv. We're gradually transitioning from dev_priv to i915, so I'm
not thrilled about adding new dev_priv.
BR,
Jani.
>
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index bc6c26818e15..773523dcd107 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd)
> static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> const void *args, int args_len)
> {
> + struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
> const char *cmd_name;
> int i, pos = 0;
> char buffer[64];
> @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> else
> BUF_PRINT("(%02X)", cmd);
>
> - WARN_ON(pos >= sizeof(buffer) - 1);
> + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> #undef BUF_PRINT
>
> DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
> @@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
> static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
> void *response, int response_len)
> {
> + struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
> const char *cmd_status;
> u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
> u8 status;
> @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
> BUF_PRINT(" %02X", ((u8 *)response)[i]);
> }
>
> - WARN_ON(pos >= sizeof(buffer) - 1);
> + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> #undef BUF_PRINT
>
> DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
> @@ -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> {
> + struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
> struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
> const struct drm_display_mode *adjusted_mode =
> &crtc_state->hw.adjusted_mode;
> @@ -1106,7 +1109,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> HDMI_QUANTIZATION_RANGE_FULL);
>
> ret = hdmi_avi_infoframe_check(frame);
> - if (WARN_ON(ret))
> + if (drm_WARN_ON(&dev_priv->drm, ret))
> return false;
>
> return true;
> @@ -1115,6 +1118,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> const struct intel_crtc_state *crtc_state)
> {
> + struct drm_i915_private *dev_priv = to_i915(intel_sdvo->base.base.dev);
> u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> const union hdmi_infoframe *frame = &crtc_state->infoframes.avi;
> ssize_t len;
> @@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
> return true;
>
> - if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> + if (drm_WARN_ON(&dev_priv->drm,
> + frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> return false;
>
> len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
> - if (WARN_ON(len < 0))
> + if (drm_WARN_ON(&dev_priv->drm, len < 0))
> return false;
>
> return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
> @@ -1237,6 +1242,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>
> static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> {
> + struct drm_i915_private *dev_priv = to_i915(pipe_config->uapi.crtc->dev);
> unsigned dotclock = pipe_config->port_clock;
> struct dpll *clock = &pipe_config->dpll;
>
> @@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> clock->m1 = 12;
> clock->m2 = 8;
> } else {
> - WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
> + drm_WARN(&dev_priv->drm, 1,
> + "SDVO TV clock out of range: %i\n", dotclock);
> }
>
> pipe_config->clock_set = true;
> @@ -2293,7 +2300,7 @@ intel_sdvo_connector_atomic_get_property(struct drm_connector *connector,
> return 0;
> }
>
> - WARN_ON(1);
> + drm_WARN_ON(connector->dev, 1);
> *val = 0;
> } else if (property == intel_sdvo_connector->top ||
> property == intel_sdvo_connector->bottom)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
2020-05-08 6:48 ` Jani Nikula
@ 2020-05-08 7:23 ` Laxminarayan Bharadiya, Pankaj
2020-05-19 13:41 ` Jani Nikula
0 siblings, 1 reply; 14+ messages in thread
From: Laxminarayan Bharadiya, Pankaj @ 2020-05-08 7:23 UTC (permalink / raw)
To: Jani Nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen, Vivi,
Rodrigo, David Airlie, Ville Syrjälä,
Chris Wilson, Deak, Imre, Maarten Lankhorst
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 08 May 2020 12:19
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> David Airlie <airlied@linux.ie>; Ville Syrjälä <ville.syrjala@linux.intel.com>; Chris
> Wilson <chris@chris-wilson.co.uk>; Deak, Imre <imre.deak@intel.com>;
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Laxminarayan
> Bharadiya, Pankaj <pankaj.laxminarayan.bharadiya@intel.com>
> Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over
> WARN*
>
> On Mon, 04 May 2020, Pankaj Bharadiya
> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> > struct drm_device specific drm_WARN* macros include device information
> > in the backtrace, so we know what device the warnings originate from.
> >
> > Prefer drm_WARN* over WARN* calls.
> >
> > changes since v1:
> > - Added dev_priv local variable and used it in drm_WARN* calls (Jani)
>
> In the earlier patches you're adding i915 local variable, here it's dev_priv. We're
> gradually transitioning from dev_priv to i915, so I'm not thrilled about adding
> new dev_priv.
dev_priv name is being used throughout the file. So to be consistent with rest of the
code, I used dev_priv variable in this specific file.
Shall I rename it to i915?
I used i915 or dev_priv variable name based on what variable name being
already used for struct drm_i915_private pointer in a given file.
Thanks,
Pankaj
>
> BR,
> Jani.
>
>
>
> >
> > Signed-off-by: Pankaj Bharadiya
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++++++++++++++-------
> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index bc6c26818e15..773523dcd107 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd) static
> > void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> > const void *args, int args_len) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > const char *cmd_name;
> > int i, pos = 0;
> > char buffer[64];
> > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo
> *intel_sdvo, u8 cmd,
> > else
> > BUF_PRINT("(%02X)", cmd);
> >
> > - WARN_ON(pos >= sizeof(buffer) - 1);
> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> > #undef BUF_PRINT
> >
> > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd,
> > buffer); @@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct
> > intel_sdvo *intel_sdvo, u8 cmd, static bool intel_sdvo_read_response(struct
> intel_sdvo *intel_sdvo,
> > void *response, int response_len) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > const char *cmd_status;
> > u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
> > u8 status;
> > @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct
> intel_sdvo *intel_sdvo,
> > BUF_PRINT(" %02X", ((u8 *)response)[i]);
> > }
> >
> > - WARN_ON(pos >= sizeof(buffer) - 1);
> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> > #undef BUF_PRINT
> >
> > DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
> @@
> > -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct
> intel_sdvo *intel_sdvo,
> > struct intel_crtc_state *crtc_state,
> > struct drm_connector_state
> *conn_state) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
> > const struct drm_display_mode *adjusted_mode =
> > &crtc_state->hw.adjusted_mode;
> > @@ -1106,7 +1109,7 @@ static bool
> intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> >
> HDMI_QUANTIZATION_RANGE_FULL);
> >
> > ret = hdmi_avi_infoframe_check(frame);
> > - if (WARN_ON(ret))
> > + if (drm_WARN_ON(&dev_priv->drm, ret))
> > return false;
> >
> > return true;
> > @@ -1115,6 +1118,7 @@ static bool
> > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, static bool
> intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> > const struct intel_crtc_state
> *crtc_state) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(intel_sdvo->base.base.dev);
> > u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> > const union hdmi_infoframe *frame = &crtc_state->infoframes.avi;
> > ssize_t len;
> > @@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct
> intel_sdvo *intel_sdvo,
> > intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
> > return true;
> >
> > - if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> > + if (drm_WARN_ON(&dev_priv->drm,
> > + frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> > return false;
> >
> > len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
> > - if (WARN_ON(len < 0))
> > + if (drm_WARN_ON(&dev_priv->drm, len < 0))
> > return false;
> >
> > return intel_sdvo_write_infoframe(intel_sdvo,
> > SDVO_HBUF_INDEX_AVI_IF, @@ -1237,6 +1242,7 @@
> > intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
> >
> > static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state
> > *pipe_config) {
> > + struct drm_i915_private *dev_priv =
> > +to_i915(pipe_config->uapi.crtc->dev);
> > unsigned dotclock = pipe_config->port_clock;
> > struct dpll *clock = &pipe_config->dpll;
> >
> > @@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct
> intel_crtc_state *pipe_config)
> > clock->m1 = 12;
> > clock->m2 = 8;
> > } else {
> > - WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
> > + drm_WARN(&dev_priv->drm, 1,
> > + "SDVO TV clock out of range: %i\n", dotclock);
> > }
> >
> > pipe_config->clock_set = true;
> > @@ -2293,7 +2300,7 @@ intel_sdvo_connector_atomic_get_property(struct
> drm_connector *connector,
> > return 0;
> > }
> >
> > - WARN_ON(1);
> > + drm_WARN_ON(connector->dev, 1);
> > *val = 0;
> > } else if (property == intel_sdvo_connector->top ||
> > property == intel_sdvo_connector->bottom)
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
2020-05-08 7:23 ` Laxminarayan Bharadiya, Pankaj
@ 2020-05-19 13:41 ` Jani Nikula
2020-05-19 14:03 ` Laxminarayan Bharadiya, Pankaj
0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-05-19 13:41 UTC (permalink / raw)
To: Laxminarayan Bharadiya, Pankaj, daniel, intel-gfx, dri-devel,
Joonas Lahtinen, Vivi, Rodrigo, David Airlie,
Ville Syrjälä,
Chris Wilson, Deak, Imre, Maarten Lankhorst
On Fri, 08 May 2020, "Laxminarayan Bharadiya, Pankaj" <pankaj.laxminarayan.bharadiya@intel.com> wrote:
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: 08 May 2020 12:19
>> To: Laxminarayan Bharadiya, Pankaj
>> <pankaj.laxminarayan.bharadiya@intel.com>; daniel@ffwll.ch; intel-
>> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas Lahtinen
>> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>> David Airlie <airlied@linux.ie>; Ville Syrjälä <ville.syrjala@linux.intel.com>; Chris
>> Wilson <chris@chris-wilson.co.uk>; Deak, Imre <imre.deak@intel.com>;
>> Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Laxminarayan
>> Bharadiya, Pankaj <pankaj.laxminarayan.bharadiya@intel.com>
>> Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over
>> WARN*
>>
>> On Mon, 04 May 2020, Pankaj Bharadiya
>> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
>> > struct drm_device specific drm_WARN* macros include device information
>> > in the backtrace, so we know what device the warnings originate from.
>> >
>> > Prefer drm_WARN* over WARN* calls.
>> >
>> > changes since v1:
>> > - Added dev_priv local variable and used it in drm_WARN* calls (Jani)
>>
>> In the earlier patches you're adding i915 local variable, here it's dev_priv. We're
>> gradually transitioning from dev_priv to i915, so I'm not thrilled about adding
>> new dev_priv.
>
> dev_priv name is being used throughout the file. So to be consistent with rest of the
> code, I used dev_priv variable in this specific file.
>
> Shall I rename it to i915?
>
> I used i915 or dev_priv variable name based on what variable name being
> already used for struct drm_i915_private pointer in a given file.
I understand your reasoning. However with i915 I've preferred to switch
when possible.
Regardless, pushed the series now. Thanks for the patches, and sorry for
the delay.
BR,
Jani.
>
> Thanks,
> Pankaj
>
>>
>> BR,
>> Jani.
>>
>>
>>
>> >
>> > Signed-off-by: Pankaj Bharadiya
>> > <pankaj.laxminarayan.bharadiya@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_sdvo.c | 21 ++++++++++++++-------
>> > 1 file changed, 14 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > index bc6c26818e15..773523dcd107 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd) static
>> > void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>> > const void *args, int args_len) {
>> > + struct drm_i915_private *dev_priv =
>> > +to_i915(intel_sdvo->base.base.dev);
>> > const char *cmd_name;
>> > int i, pos = 0;
>> > char buffer[64];
>> > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct intel_sdvo
>> *intel_sdvo, u8 cmd,
>> > else
>> > BUF_PRINT("(%02X)", cmd);
>> >
>> > - WARN_ON(pos >= sizeof(buffer) - 1);
>> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
>> > #undef BUF_PRINT
>> >
>> > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd,
>> > buffer); @@ -533,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct
>> > intel_sdvo *intel_sdvo, u8 cmd, static bool intel_sdvo_read_response(struct
>> intel_sdvo *intel_sdvo,
>> > void *response, int response_len) {
>> > + struct drm_i915_private *dev_priv =
>> > +to_i915(intel_sdvo->base.base.dev);
>> > const char *cmd_status;
>> > u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
>> > u8 status;
>> > @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct
>> intel_sdvo *intel_sdvo,
>> > BUF_PRINT(" %02X", ((u8 *)response)[i]);
>> > }
>> >
>> > - WARN_ON(pos >= sizeof(buffer) - 1);
>> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
>> > #undef BUF_PRINT
>> >
>> > DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
>> @@
>> > -1081,6 +1083,7 @@ static bool intel_sdvo_compute_avi_infoframe(struct
>> intel_sdvo *intel_sdvo,
>> > struct intel_crtc_state *crtc_state,
>> > struct drm_connector_state
>> *conn_state) {
>> > + struct drm_i915_private *dev_priv =
>> > +to_i915(intel_sdvo->base.base.dev);
>> > struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
>> > const struct drm_display_mode *adjusted_mode =
>> > &crtc_state->hw.adjusted_mode;
>> > @@ -1106,7 +1109,7 @@ static bool
>> intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
>> >
>> HDMI_QUANTIZATION_RANGE_FULL);
>> >
>> > ret = hdmi_avi_infoframe_check(frame);
>> > - if (WARN_ON(ret))
>> > + if (drm_WARN_ON(&dev_priv->drm, ret))
>> > return false;
>> >
>> > return true;
>> > @@ -1115,6 +1118,7 @@ static bool
>> > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo, static bool
>> intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
>> > const struct intel_crtc_state
>> *crtc_state) {
>> > + struct drm_i915_private *dev_priv =
>> > +to_i915(intel_sdvo->base.base.dev);
>> > u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
>> > const union hdmi_infoframe *frame = &crtc_state->infoframes.avi;
>> > ssize_t len;
>> > @@ -1123,11 +1127,12 @@ static bool intel_sdvo_set_avi_infoframe(struct
>> intel_sdvo *intel_sdvo,
>> > intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
>> > return true;
>> >
>> > - if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
>> > + if (drm_WARN_ON(&dev_priv->drm,
>> > + frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
>> > return false;
>> >
>> > len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
>> > - if (WARN_ON(len < 0))
>> > + if (drm_WARN_ON(&dev_priv->drm, len < 0))
>> > return false;
>> >
>> > return intel_sdvo_write_infoframe(intel_sdvo,
>> > SDVO_HBUF_INDEX_AVI_IF, @@ -1237,6 +1242,7 @@
>> > intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
>> >
>> > static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state
>> > *pipe_config) {
>> > + struct drm_i915_private *dev_priv =
>> > +to_i915(pipe_config->uapi.crtc->dev);
>> > unsigned dotclock = pipe_config->port_clock;
>> > struct dpll *clock = &pipe_config->dpll;
>> >
>> > @@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct
>> intel_crtc_state *pipe_config)
>> > clock->m1 = 12;
>> > clock->m2 = 8;
>> > } else {
>> > - WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
>> > + drm_WARN(&dev_priv->drm, 1,
>> > + "SDVO TV clock out of range: %i\n", dotclock);
>> > }
>> >
>> > pipe_config->clock_set = true;
>> > @@ -2293,7 +2300,7 @@ intel_sdvo_connector_atomic_get_property(struct
>> drm_connector *connector,
>> > return 0;
>> > }
>> >
>> > - WARN_ON(1);
>> > + drm_WARN_ON(connector->dev, 1);
>> > *val = 0;
>> > } else if (property == intel_sdvo_connector->top ||
>> > property == intel_sdvo_connector->bottom)
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over WARN*
2020-05-19 13:41 ` Jani Nikula
@ 2020-05-19 14:03 ` Laxminarayan Bharadiya, Pankaj
0 siblings, 0 replies; 14+ messages in thread
From: Laxminarayan Bharadiya, Pankaj @ 2020-05-19 14:03 UTC (permalink / raw)
To: Jani Nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen, Vivi,
Rodrigo, David Airlie, Ville Syrjälä,
Chris Wilson, Deak, Imre, Maarten Lankhorst
> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 19 May 2020 19:12
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> David Airlie <airlied@linux.ie>; Ville Syrjälä <ville.syrjala@linux.intel.com>; Chris
> Wilson <chris@chris-wilson.co.uk>; Deak, Imre <imre.deak@intel.com>;
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Subject: RE: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN* over
> WARN*
>
> On Fri, 08 May 2020, "Laxminarayan Bharadiya, Pankaj"
> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> >> -----Original Message-----
> >> From: Jani Nikula <jani.nikula@linux.intel.com>
> >> Sent: 08 May 2020 12:19
> >> To: Laxminarayan Bharadiya, Pankaj
> >> <pankaj.laxminarayan.bharadiya@intel.com>; daniel@ffwll.ch; intel-
> >> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Joonas
> >> Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo
> >> <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Ville
> >> Syrjälä <ville.syrjala@linux.intel.com>; Chris Wilson
> >> <chris@chris-wilson.co.uk>; Deak, Imre <imre.deak@intel.com>; Maarten
> >> Lankhorst <maarten.lankhorst@linux.intel.com>; Laxminarayan
> >> Bharadiya, Pankaj <pankaj.laxminarayan.bharadiya@intel.com>
> >> Subject: Re: [PATCH v2 3/9] drm/i915/display/sdvo: Prefer drm_WARN*
> >> over
> >> WARN*
> >>
> >> On Mon, 04 May 2020, Pankaj Bharadiya
> >> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> >> > struct drm_device specific drm_WARN* macros include device
> >> > information in the backtrace, so we know what device the warnings
> originate from.
> >> >
> >> > Prefer drm_WARN* over WARN* calls.
> >> >
> >> > changes since v1:
> >> > - Added dev_priv local variable and used it in drm_WARN* calls
> >> > (Jani)
> >>
> >> In the earlier patches you're adding i915 local variable, here it's
> >> dev_priv. We're gradually transitioning from dev_priv to i915, so I'm
> >> not thrilled about adding new dev_priv.
> >
> > dev_priv name is being used throughout the file. So to be consistent
> > with rest of the code, I used dev_priv variable in this specific file.
> >
> > Shall I rename it to i915?
> >
> > I used i915 or dev_priv variable name based on what variable name
> > being already used for struct drm_i915_private pointer in a given file.
>
> I understand your reasoning. However with i915 I've preferred to switch when
> possible.
>
> Regardless, pushed the series now. Thanks for the patches, and sorry for the
> delay.
Thank you Jani.
Will you please review - https://patchwork.freedesktop.org/series/75265/#rev2
Thanks,
Pankaj
>
> BR,
> Jani.
>
>
>
> >
> > Thanks,
> > Pankaj
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >>
> >> >
> >> > Signed-off-by: Pankaj Bharadiya
> >> > <pankaj.laxminarayan.bharadiya@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 21
> >> > ++++++++++++++-------
> >> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > index bc6c26818e15..773523dcd107 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > @@ -411,6 +411,7 @@ static const char *sdvo_cmd_name(u8 cmd)
> >> > static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> >> > const void *args, int args_len) {
> >> > + struct drm_i915_private *dev_priv =
> >> > +to_i915(intel_sdvo->base.base.dev);
> >> > const char *cmd_name;
> >> > int i, pos = 0;
> >> > char buffer[64];
> >> > @@ -431,7 +432,7 @@ static void intel_sdvo_debug_write(struct
> >> > intel_sdvo
> >> *intel_sdvo, u8 cmd,
> >> > else
> >> > BUF_PRINT("(%02X)", cmd);
> >> >
> >> > - WARN_ON(pos >= sizeof(buffer) - 1);
> >> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> >> > #undef BUF_PRINT
> >> >
> >> > DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd,
> >> > buffer); @@ -533,6 +534,7 @@ static bool
> >> > intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd, static
> >> > bool intel_sdvo_read_response(struct
> >> intel_sdvo *intel_sdvo,
> >> > void *response, int response_len) {
> >> > + struct drm_i915_private *dev_priv =
> >> > +to_i915(intel_sdvo->base.base.dev);
> >> > const char *cmd_status;
> >> > u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
> >> > u8 status;
> >> > @@ -597,7 +599,7 @@ static bool intel_sdvo_read_response(struct
> >> intel_sdvo *intel_sdvo,
> >> > BUF_PRINT(" %02X", ((u8 *)response)[i]);
> >> > }
> >> >
> >> > - WARN_ON(pos >= sizeof(buffer) - 1);
> >> > + drm_WARN_ON(&dev_priv->drm, pos >= sizeof(buffer) - 1);
> >> > #undef BUF_PRINT
> >> >
> >> > DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
> >> @@
> >> > -1081,6 +1083,7 @@ static bool
> >> > intel_sdvo_compute_avi_infoframe(struct
> >> intel_sdvo *intel_sdvo,
> >> > struct intel_crtc_state *crtc_state,
> >> > struct drm_connector_state
> >> *conn_state) {
> >> > + struct drm_i915_private *dev_priv =
> >> > +to_i915(intel_sdvo->base.base.dev);
> >> > struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
> >> > const struct drm_display_mode *adjusted_mode =
> >> > &crtc_state->hw.adjusted_mode;
> >> > @@ -1106,7 +1109,7 @@ static bool
> >> intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> >> >
> >> HDMI_QUANTIZATION_RANGE_FULL);
> >> >
> >> > ret = hdmi_avi_infoframe_check(frame);
> >> > - if (WARN_ON(ret))
> >> > + if (drm_WARN_ON(&dev_priv->drm, ret))
> >> > return false;
> >> >
> >> > return true;
> >> > @@ -1115,6 +1118,7 @@ static bool
> >> > intel_sdvo_compute_avi_infoframe(struct intel_sdvo *intel_sdvo,
> >> > static bool
> >> intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> >> > const struct intel_crtc_state
> >> *crtc_state) {
> >> > + struct drm_i915_private *dev_priv =
> >> > +to_i915(intel_sdvo->base.base.dev);
> >> > u8 sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> >> > const union hdmi_infoframe *frame = &crtc_state->infoframes.avi;
> >> > ssize_t len;
> >> > @@ -1123,11 +1127,12 @@ static bool
> >> > intel_sdvo_set_avi_infoframe(struct
> >> intel_sdvo *intel_sdvo,
> >> > intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI)) == 0)
> >> > return true;
> >> >
> >> > - if (WARN_ON(frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> >> > + if (drm_WARN_ON(&dev_priv->drm,
> >> > + frame->any.type != HDMI_INFOFRAME_TYPE_AVI))
> >> > return false;
> >> >
> >> > len = hdmi_infoframe_pack_only(frame, sdvo_data, sizeof(sdvo_data));
> >> > - if (WARN_ON(len < 0))
> >> > + if (drm_WARN_ON(&dev_priv->drm, len < 0))
> >> > return false;
> >> >
> >> > return intel_sdvo_write_infoframe(intel_sdvo,
> >> > SDVO_HBUF_INDEX_AVI_IF, @@ -1237,6 +1242,7 @@
> >> > intel_sdvo_get_preferred_input_mode(struct intel_sdvo *intel_sdvo,
> >> >
> >> > static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state
> >> > *pipe_config) {
> >> > + struct drm_i915_private *dev_priv =
> >> > +to_i915(pipe_config->uapi.crtc->dev);
> >> > unsigned dotclock = pipe_config->port_clock;
> >> > struct dpll *clock = &pipe_config->dpll;
> >> >
> >> > @@ -1257,7 +1263,8 @@ static void i9xx_adjust_sdvo_tv_clock(struct
> >> intel_crtc_state *pipe_config)
> >> > clock->m1 = 12;
> >> > clock->m2 = 8;
> >> > } else {
> >> > - WARN(1, "SDVO TV clock out of range: %i\n", dotclock);
> >> > + drm_WARN(&dev_priv->drm, 1,
> >> > + "SDVO TV clock out of range: %i\n", dotclock);
> >> > }
> >> >
> >> > pipe_config->clock_set = true;
> >> > @@ -2293,7 +2300,7 @@
> >> > intel_sdvo_connector_atomic_get_property(struct
> >> drm_connector *connector,
> >> > return 0;
> >> > }
> >> >
> >> > - WARN_ON(1);
> >> > + drm_WARN_ON(connector->dev, 1);
> >> > *val = 0;
> >> > } else if (property == intel_sdvo_connector->top ||
> >> > property == intel_sdvo_connector->bottom)
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
>
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/9] drm/i915/display/tc: Prefer drm_WARN_ON over WARN_ON
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (2 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 3/9] drm/i915/display/sdvo: " Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 5/9] drm/i915/gem: Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Imre Deak,
José Roberto de Souza, Ville Syrjälä,
Lucas De Marchi, Pankaj Bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Conversion is done with below sementic patch:
@@
identifier func, T;
@@
func(...) {
...
struct drm_i915_private *T = ...;
<+...
-WARN_ON(
+drm_WARN_ON(&T->drm,
...)
...+>
}
@@
identifier func, T;
@@
func(struct intel_digital_port *T,...) {
+struct drm_i915_private *i915 = to_i915(T->base.base.dev);
<+...
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
...+>
}
changes since v1:
- Add i915 local variable and use it in drm_WARN_ON (Jani)
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/display/intel_tc.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index d3bd5e798fbc..a2ffc991bc0e 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -360,12 +360,12 @@ static void icl_tc_phy_connect(struct intel_digital_port *dig_port,
}
if (!icl_tc_phy_set_safe_mode(dig_port, false) &&
- !WARN_ON(dig_port->tc_legacy_port))
+ !drm_WARN_ON(&i915->drm, dig_port->tc_legacy_port))
goto out_set_tbt_alt_mode;
max_lanes = intel_tc_port_fia_max_lane_count(dig_port);
if (dig_port->tc_legacy_port) {
- WARN_ON(max_lanes != 4);
+ drm_WARN_ON(&i915->drm, max_lanes != 4);
dig_port->tc_mode = TC_PORT_LEGACY;
return;
@@ -445,18 +445,20 @@ static bool icl_tc_phy_is_connected(struct intel_digital_port *dig_port)
static enum tc_port_mode
intel_tc_port_get_current_mode(struct intel_digital_port *dig_port)
{
+ struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
u32 live_status_mask = tc_port_live_status_mask(dig_port);
bool in_safe_mode = icl_tc_phy_is_in_safe_mode(dig_port);
enum tc_port_mode mode;
- if (in_safe_mode || WARN_ON(!icl_tc_phy_status_complete(dig_port)))
+ if (in_safe_mode ||
+ drm_WARN_ON(&i915->drm, !icl_tc_phy_status_complete(dig_port)))
return TC_PORT_TBT_ALT;
mode = dig_port->tc_legacy_port ? TC_PORT_LEGACY : TC_PORT_DP_ALT;
if (live_status_mask) {
enum tc_port_mode live_mode = fls(live_status_mask) - 1;
- if (!WARN_ON(live_mode == TC_PORT_TBT_ALT))
+ if (!drm_WARN_ON(&i915->drm, live_mode == TC_PORT_TBT_ALT))
mode = live_mode;
}
@@ -505,7 +507,9 @@ static void
intel_tc_port_link_init_refcount(struct intel_digital_port *dig_port,
int refcount)
{
- WARN_ON(dig_port->tc_link_refcount);
+ struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+
+ drm_WARN_ON(&i915->drm, dig_port->tc_link_refcount);
dig_port->tc_link_refcount = refcount;
}
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 5/9] drm/i915/gem: Prefer drm_WARN* over WARN*
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (3 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 4/9] drm/i915/display/tc: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 6/9] drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie, Chris Wilson, Matthew Auld,
Tvrtko Ursulin, Mika Kuoppala, Jon Bloomfield, Pankaj Bharadiya,
Abdiel Janulgue
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN* at places where struct drm_device pointer
can be extracted.
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 3 ++-
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index cce7df231cb9..cf9f0e078202 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1574,7 +1574,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
err = i915_vma_bind(target->vma,
target->vma->obj->cache_level,
PIN_GLOBAL, NULL);
- if (WARN_ONCE(err,
+ if (drm_WARN_ONCE(&i915->drm, err,
"Unexpected failure to bind target VMA!"))
return err;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 7fe9831aa9ba..4c1c7232b024 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -27,7 +27,8 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
void *dst;
int i;
- if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
+ if (drm_WARN_ON(obj->base.dev,
+ i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
/*
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 7ffd7afeb7a5..8b0708708671 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -235,7 +235,7 @@ i915_gem_userptr_init__mmu_notifier(struct drm_i915_gem_object *obj,
if (flags & I915_USERPTR_UNSYNCHRONIZED)
return capable(CAP_SYS_ADMIN) ? 0 : -EPERM;
- if (WARN_ON(obj->userptr.mm == NULL))
+ if (drm_WARN_ON(obj->base.dev, obj->userptr.mm == NULL))
return -EINVAL;
mn = i915_mmu_notifier_find(obj->userptr.mm);
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 6/9] drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (4 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 5/9] drm/i915/gem: Prefer drm_WARN* over WARN* Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 7/9] drm/i915/pmu: " Pankaj Bharadiya
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
changes since v1:
- Add parentheses around the dev_priv macro argument (Jani)
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6af69555733e..9fdf4bcd06e1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1649,7 +1649,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_DISPLAY(dev_priv) (INTEL_INFO(dev_priv)->pipe_mask != 0)
/* Only valid when HAS_DISPLAY() is true */
-#define INTEL_DISPLAY_ENABLED(dev_priv) (WARN_ON(!HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
+#define INTEL_DISPLAY_ENABLED(dev_priv) \
+ (drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !i915_modparams.disable_display)
static inline bool intel_vtd_active(void)
{
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 7/9] drm/i915/pmu: Prefer drm_WARN_ON over WARN_ON
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (5 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 6/9] drm/i915/i915_drv: Prefer drm_WARN_ON over WARN_ON Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:15 ` [PATCH v2 8/9] drm/i915/pm: " Pankaj Bharadiya
2020-05-04 18:16 ` [PATCH v2 9/9] drm/i915/runtime_pm: Prefer drm_WARN* over WARN* Pankaj Bharadiya
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/i915_pmu.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e991a707bdb7..f6f44ad5e335 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -441,7 +441,11 @@ static u64 count_interrupts(struct drm_i915_private *i915)
static void i915_pmu_event_destroy(struct perf_event *event)
{
- WARN_ON(event->parent);
+ struct drm_i915_private *i915 =
+ container_of(event->pmu, typeof(*i915), pmu.base);
+
+ drm_WARN_ON(&i915->drm, event->parent);
+
module_put(THIS_MODULE);
}
@@ -1058,8 +1062,10 @@ static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
{
- WARN_ON(pmu->cpuhp.slot == CPUHP_INVALID);
- WARN_ON(cpuhp_state_remove_instance(pmu->cpuhp.slot, &pmu->cpuhp.node));
+ struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
+
+ drm_WARN_ON(&i915->drm, pmu->cpuhp.slot == CPUHP_INVALID);
+ drm_WARN_ON(&i915->drm, cpuhp_state_remove_instance(pmu->cpuhp.slot, &pmu->cpuhp.node));
cpuhp_remove_multi_state(pmu->cpuhp.slot);
pmu->cpuhp.slot = CPUHP_INVALID;
}
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 8/9] drm/i915/pm: Prefer drm_WARN_ON over WARN_ON
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (6 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 7/9] drm/i915/pmu: " Pankaj Bharadiya
@ 2020-05-04 18:15 ` Pankaj Bharadiya
2020-05-04 18:16 ` [PATCH v2 9/9] drm/i915/runtime_pm: Prefer drm_WARN* over WARN* Pankaj Bharadiya
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:15 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN_ON over WARN_ON.
Conversion is done with below sementic patch:
@@
identifier func, T;
@@
func(...) {
...
struct intel_crtc *T = ...;
+struct drm_i915_private *dev_priv = to_i915(T->base.dev);
<+...
-WARN_ON(
+drm_WARN_ON(&dev_priv->drm,
...)
...+>
}
@@
identifier func, T;
@@
func(struct intel_crtc_state *T,...) {
+struct drm_i915_private *dev_priv = to_i915(T->uapi.crtc->dev);
<+...
-WARN_ON(
+drm_WARN_ON(&dev_priv->drm,
...)
...+>
}
changes since v1:
- Added dev_priv local variable and used it in drm_WARN_ON calls (Jani)
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 63 ++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bfb180fe8047..7e8da6c5bfb9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1436,6 +1436,7 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
struct g4x_wm_state *intermediate = &new_crtc_state->wm.g4x.intermediate;
const struct g4x_wm_state *optimal = &new_crtc_state->wm.g4x.optimal;
struct intel_atomic_state *intel_state =
@@ -1464,8 +1465,8 @@ static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
max(optimal->wm.plane[plane_id],
active->wm.plane[plane_id]);
- WARN_ON(intermediate->wm.plane[plane_id] >
- g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
+ drm_WARN_ON(&dev_priv->drm, intermediate->wm.plane[plane_id] >
+ g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
}
intermediate->sr.plane = max(optimal->sr.plane,
@@ -1482,21 +1483,25 @@ static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
intermediate->hpll.fbc = max(optimal->hpll.fbc,
active->hpll.fbc);
- WARN_ON((intermediate->sr.plane >
- g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
- intermediate->sr.cursor >
- g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
- intermediate->cxsr);
- WARN_ON((intermediate->sr.plane >
- g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
- intermediate->sr.cursor >
- g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
- intermediate->hpll_en);
-
- WARN_ON(intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
- intermediate->fbc_en && intermediate->cxsr);
- WARN_ON(intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
- intermediate->fbc_en && intermediate->hpll_en);
+ drm_WARN_ON(&dev_priv->drm,
+ (intermediate->sr.plane >
+ g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
+ intermediate->sr.cursor >
+ g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
+ intermediate->cxsr);
+ drm_WARN_ON(&dev_priv->drm,
+ (intermediate->sr.plane >
+ g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
+ intermediate->sr.cursor >
+ g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
+ intermediate->hpll_en);
+
+ drm_WARN_ON(&dev_priv->drm,
+ intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
+ intermediate->fbc_en && intermediate->cxsr);
+ drm_WARN_ON(&dev_priv->drm,
+ intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
+ intermediate->fbc_en && intermediate->hpll_en);
out:
/*
@@ -1680,6 +1685,7 @@ static bool vlv_need_sprite0_fifo_workaround(unsigned int active_planes)
static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
const struct g4x_pipe_wm *raw =
&crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2];
struct vlv_fifo_state *fifo_state = &crtc_state->wm.vlv.fifo_state;
@@ -1748,11 +1754,11 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
fifo_left -= plane_extra;
}
- WARN_ON(active_planes != 0 && fifo_left != 0);
+ drm_WARN_ON(&dev_priv->drm, active_planes != 0 && fifo_left != 0);
/* give it all to the first plane if none are active */
if (active_planes == 0) {
- WARN_ON(fifo_left != fifo_size);
+ drm_WARN_ON(&dev_priv->drm, fifo_left != fifo_size);
fifo_state->plane[PLANE_PRIMARY] = fifo_left;
}
@@ -4180,11 +4186,13 @@ static uint_fixed_16_16_t
skl_plane_downscale_amount(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
u32 src_w, src_h, dst_w, dst_h;
uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
uint_fixed_16_16_t downscale_h, downscale_w;
- if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
+ if (drm_WARN_ON(&dev_priv->drm,
+ !intel_wm_plane_visible(crtc_state, plane_state)))
return u32_to_fixed16(0);
/*
@@ -4836,6 +4844,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
static uint_fixed_16_16_t
intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
u32 pixel_rate;
u32 crtc_htotal;
uint_fixed_16_16_t linetime_us;
@@ -4845,7 +4854,7 @@ intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
pixel_rate = crtc_state->pixel_rate;
- if (WARN_ON(pixel_rate == 0))
+ if (drm_WARN_ON(&dev_priv->drm, pixel_rate == 0))
return u32_to_fixed16(0);
crtc_htotal = crtc_state->hw.adjusted_mode.crtc_htotal;
@@ -4858,11 +4867,13 @@ static u32
skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
u64 adjusted_pixel_rate;
uint_fixed_16_16_t downscale_amount;
/* Shouldn't reach here on disabled planes... */
- if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
+ if (drm_WARN_ON(&dev_priv->drm,
+ !intel_wm_plane_visible(crtc_state, plane_state)))
return 0;
/*
@@ -5281,6 +5292,7 @@ static int skl_build_plane_wm(struct intel_crtc_state *crtc_state,
static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
enum plane_id plane_id = to_intel_plane(plane_state->uapi.plane)->id;
int ret;
@@ -5292,9 +5304,10 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
const struct drm_framebuffer *fb = plane_state->hw.fb;
enum plane_id y_plane_id = plane_state->planar_linked_plane->id;
- WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state));
- WARN_ON(!fb->format->is_yuv ||
- fb->format->num_planes == 1);
+ drm_WARN_ON(&dev_priv->drm,
+ !intel_wm_plane_visible(crtc_state, plane_state));
+ drm_WARN_ON(&dev_priv->drm, !fb->format->is_yuv ||
+ fb->format->num_planes == 1);
ret = skl_build_plane_wm_single(crtc_state, plane_state,
y_plane_id, 0);
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 9/9] drm/i915/runtime_pm: Prefer drm_WARN* over WARN*
2020-05-04 18:15 [PATCH v2 0/9] Prefer drm_WARN* over WARN* Pankaj Bharadiya
` (7 preceding siblings ...)
2020-05-04 18:15 ` [PATCH v2 8/9] drm/i915/pm: " Pankaj Bharadiya
@ 2020-05-04 18:16 ` Pankaj Bharadiya
8 siblings, 0 replies; 14+ messages in thread
From: Pankaj Bharadiya @ 2020-05-04 18:16 UTC (permalink / raw)
To: jani.nikula, daniel, intel-gfx, dri-devel, Joonas Lahtinen,
Rodrigo Vivi, David Airlie
Cc: pankaj.laxminarayan.bharadiya
struct drm_device specific drm_WARN* macros include device information
in the backtrace, so we know what device the warnings originate from.
Prefer drm_WARN* over WARN*.
Conversion is done with below semantic patch:
@@
identifier func, T;
@@
func(struct intel_runtime_pm *T,...) {
+ struct drm_i915_private *i915 = container_of(T, struct drm_i915_private, runtime_pm);
<+...
(
-WARN(
+drm_WARN(&i915->drm,
...)
|
-WARN_ON(
+drm_WARN_ON(&i915->drm,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(&i915->drm,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(&i915->drm,
...)
)
...+>
}
Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 39 ++++++++++++++++++-------
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..31ccd0559c55 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -116,6 +116,9 @@ track_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm)
static void untrack_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm,
depot_stack_handle_t stack)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+ struct drm_i915_private,
+ runtime_pm);
unsigned long flags, n;
bool found = false;
@@ -134,9 +137,9 @@ static void untrack_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm,
}
spin_unlock_irqrestore(&rpm->debug.lock, flags);
- if (WARN(!found,
- "Unmatched wakeref (tracking %lu), count %u\n",
- rpm->debug.count, atomic_read(&rpm->wakeref_count))) {
+ if (drm_WARN(&i915->drm, !found,
+ "Unmatched wakeref (tracking %lu), count %u\n",
+ rpm->debug.count, atomic_read(&rpm->wakeref_count))) {
char *buf;
buf = kmalloc(PAGE_SIZE, GFP_NOWAIT | __GFP_NOWARN);
@@ -355,10 +358,14 @@ intel_runtime_pm_release(struct intel_runtime_pm *rpm, int wakelock)
static intel_wakeref_t __intel_runtime_pm_get(struct intel_runtime_pm *rpm,
bool wakelock)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+ struct drm_i915_private,
+ runtime_pm);
int ret;
ret = pm_runtime_get_sync(rpm->kdev);
- WARN_ONCE(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+ drm_WARN_ONCE(&i915->drm, ret < 0,
+ "pm_runtime_get_sync() failed: %d\n", ret);
intel_runtime_pm_acquire(rpm, wakelock);
@@ -539,6 +546,9 @@ void intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_wakeref_t wref)
*/
void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+ struct drm_i915_private,
+ runtime_pm);
struct device *kdev = rpm->kdev;
/*
@@ -565,7 +575,8 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
pm_runtime_dont_use_autosuspend(kdev);
ret = pm_runtime_get_sync(kdev);
- WARN(ret < 0, "pm_runtime_get_sync() failed: %d\n", ret);
+ drm_WARN(&i915->drm, ret < 0,
+ "pm_runtime_get_sync() failed: %d\n", ret);
} else {
pm_runtime_use_autosuspend(kdev);
}
@@ -580,11 +591,14 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
void intel_runtime_pm_disable(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+ struct drm_i915_private,
+ runtime_pm);
struct device *kdev = rpm->kdev;
/* Transfer rpm ownership back to core */
- WARN(pm_runtime_get_sync(kdev) < 0,
- "Failed to pass rpm ownership back to core\n");
+ drm_WARN(&i915->drm, pm_runtime_get_sync(kdev) < 0,
+ "Failed to pass rpm ownership back to core\n");
pm_runtime_dont_use_autosuspend(kdev);
@@ -594,12 +608,15 @@ void intel_runtime_pm_disable(struct intel_runtime_pm *rpm)
void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm)
{
+ struct drm_i915_private *i915 = container_of(rpm,
+ struct drm_i915_private,
+ runtime_pm);
int count = atomic_read(&rpm->wakeref_count);
- WARN(count,
- "i915 raw-wakerefs=%d wakelocks=%d on cleanup\n",
- intel_rpm_raw_wakeref_count(count),
- intel_rpm_wakelock_count(count));
+ drm_WARN(&i915->drm, count,
+ "i915 raw-wakerefs=%d wakelocks=%d on cleanup\n",
+ intel_rpm_raw_wakeref_count(count),
+ intel_rpm_wakelock_count(count));
untrack_all_intel_runtime_pm_wakerefs(rpm);
}
--
2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread