* [PATCH 0/2] tonga_get_evv_voltage error handling fixes @ 2016-04-14 20:10 Moritz Kühner 2016-04-14 20:10 ` [PATCH 1/2] drm/amd/powerplay/hwmgr: prevent VDDC from exceeding 2V Moritz Kühner 2016-04-14 20:10 ` [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage Moritz Kühner 0 siblings, 2 replies; 4+ messages in thread From: Moritz Kühner @ 2016-04-14 20:10 UTC (permalink / raw) To: dri-devel; +Cc: alexander.deucher, Moritz Kühner I got myself a 380X recently and started reading random mesa and kernel code in the hopes that I would find something that I can fix or improve, and something actually caught my eye. Some of the error handling in tonga_get_evv_voltage just seemed of and based on the comments I think the patches provided will do the intended thing. While I did test the patch I have to admit that i did not try what happens when I apply 2V to my card ;-). PS: This is my first submission. So... please tell me if I did something wrong. Moritz Kühner (2): drm/amd/powerplay/hwmgr: prevent VDDC from exceeding 2V drm/amd/powerplay/hwmgr: don't add invalid voltage drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 55 ++++++++++++----------- 1 file changed, 30 insertions(+), 25 deletions(-) -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] drm/amd/powerplay/hwmgr: prevent VDDC from exceeding 2V 2016-04-14 20:10 [PATCH 0/2] tonga_get_evv_voltage error handling fixes Moritz Kühner @ 2016-04-14 20:10 ` Moritz Kühner 2016-04-14 20:10 ` [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage Moritz Kühner 1 sibling, 0 replies; 4+ messages in thread From: Moritz Kühner @ 2016-04-14 20:10 UTC (permalink / raw) To: dri-devel; +Cc: alexander.deucher, Moritz Kühner If the tonga gpu is controlled by SVID2 tonga_get_evv_voltage will only print an error if the voltage exceeds 2V although a comment clearly states that it needs be less than 2V. --- drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c index 0d5d837..50afb02 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c @@ -455,8 +455,7 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr) "Error retrieving EVV voltage value!", continue); /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */ - if (vddc > 2000) - printk(KERN_ERR "[ powerplay ] Invalid VDDC value! \n"); + PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC value!", return -1); /* the voltage should not be zero nor equal to leakage ID */ if (vddc != 0 && vddc != virtual_voltage_id) { -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage 2016-04-14 20:10 [PATCH 0/2] tonga_get_evv_voltage error handling fixes Moritz Kühner 2016-04-14 20:10 ` [PATCH 1/2] drm/amd/powerplay/hwmgr: prevent VDDC from exceeding 2V Moritz Kühner @ 2016-04-14 20:10 ` Moritz Kühner 2016-04-15 0:34 ` Gustavo Padovan 1 sibling, 1 reply; 4+ messages in thread From: Moritz Kühner @ 2016-04-14 20:10 UTC (permalink / raw) To: dri-devel; +Cc: alexander.deucher, Moritz Kühner if atomctrl_get_voltage_evv_on_sclk returns non zero (fail) in the expansion of the PP_ASSERT_WITH_CODE macro the continue will actually do nothing (The macro uses a do ... while(0) as scope, which eats the continue). Based on the code I don't think this was the intent. Unfortunately fixing this requires rewriting the control flow and removing the macros. --- drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 54 +++++++++++++---------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c index 50afb02..9a5d10a 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c @@ -429,19 +429,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr) } } } - PP_ASSERT_WITH_CODE(0 == atomctrl_get_voltage_evv_on_sclk + if (0 == atomctrl_get_voltage_evv_on_sclk (hwmgr, VOLTAGE_TYPE_VDDGFX, sclk, - virtual_voltage_id, &vddgfx), - "Error retrieving EVV voltage value!", continue); - - /* need to make sure vddgfx is less than 2v or else, it could burn the ASIC. */ - PP_ASSERT_WITH_CODE((vddgfx < 2000 && vddgfx != 0), "Invalid VDDGFX value!", return -1); - - /* the voltage should not be zero nor equal to leakage ID */ - if (vddgfx != 0 && vddgfx != virtual_voltage_id) { - data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx; - data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] = virtual_voltage_id; - data->vddcgfx_leakage.count++; + virtual_voltage_id, &vddgfx)) { + /* need to make sure vddgfx is less than 2v or else, it could burn the ASIC. */ + PP_ASSERT_WITH_CODE((vddgfx < 2000 && vddgfx != 0), "Invalid VDDGFX value!", return -1); + + /* the voltage should not be zero nor equal to leakage ID */ + if (vddgfx != 0 && vddgfx != virtual_voltage_id) { + data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx; + data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] = virtual_voltage_id; + data->vddcgfx_leakage.count++; + } + } + else + { + printk("%s\n", "Error retrieving EVV voltage value!"); } } } else { @@ -449,19 +452,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr) if (0 == tonga_get_sclk_for_voltage_evv(hwmgr, pptable_info->vddc_lookup_table, virtual_voltage_id, &sclk)) { - PP_ASSERT_WITH_CODE(0 == atomctrl_get_voltage_evv_on_sclk + if (0 == atomctrl_get_voltage_evv_on_sclk (hwmgr, VOLTAGE_TYPE_VDDC, sclk, - virtual_voltage_id, &vddc), - "Error retrieving EVV voltage value!", continue); - - /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */ - PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC value!", return -1); - - /* the voltage should not be zero nor equal to leakage ID */ - if (vddc != 0 && vddc != virtual_voltage_id) { - data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc; - data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id; - data->vddc_leakage.count++; + virtual_voltage_id, &vddc)) { + /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */ + PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC value!", return -1); + + /* the voltage should not be zero nor equal to leakage ID */ + if (vddc != 0 && vddc != virtual_voltage_id) { + data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc; + data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id; + data->vddc_leakage.count++; + } + } + else + { + printk("%s\n", "Error retrieving EVV voltage value!"); } } } -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage 2016-04-14 20:10 ` [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage Moritz Kühner @ 2016-04-15 0:34 ` Gustavo Padovan 0 siblings, 0 replies; 4+ messages in thread From: Gustavo Padovan @ 2016-04-15 0:34 UTC (permalink / raw) To: Moritz Kühner; +Cc: alexander.deucher, dri-devel Hi Moritz, 2016-04-14 Moritz Kühner <kuehner.moritz@gmail.com>: > if atomctrl_get_voltage_evv_on_sclk returns non zero (fail) in the expansion > of the PP_ASSERT_WITH_CODE macro the continue will actually do nothing > (The macro uses a do ... while(0) as scope, which eats the continue). > Based on the code I don't think this was the intent. > Unfortunately fixing this requires rewriting the control flow and > removing the macros. > --- > drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 54 +++++++++++++---------- > 1 file changed, 30 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > index 50afb02..9a5d10a 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > @@ -429,19 +429,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr) > } > } > } > - PP_ASSERT_WITH_CODE(0 == atomctrl_get_voltage_evv_on_sclk > + if (0 == atomctrl_get_voltage_evv_on_sclk > (hwmgr, VOLTAGE_TYPE_VDDGFX, sclk, > - virtual_voltage_id, &vddgfx), > - "Error retrieving EVV voltage value!", continue); > - > - /* need to make sure vddgfx is less than 2v or else, it could burn the ASIC. */ > - PP_ASSERT_WITH_CODE((vddgfx < 2000 && vddgfx != 0), "Invalid VDDGFX value!", return -1); > - > - /* the voltage should not be zero nor equal to leakage ID */ > - if (vddgfx != 0 && vddgfx != virtual_voltage_id) { > - data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx; > - data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] = virtual_voltage_id; > - data->vddcgfx_leakage.count++; > + virtual_voltage_id, &vddgfx)) { > + /* need to make sure vddgfx is less than 2v or else, it could burn the ASIC. */ > + PP_ASSERT_WITH_CODE((vddgfx < 2000 && vddgfx != 0), "Invalid VDDGFX value!", return -1); > + > + /* the voltage should not be zero nor equal to leakage ID */ > + if (vddgfx != 0 && vddgfx != virtual_voltage_id) { > + data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx; > + data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] = virtual_voltage_id; > + data->vddcgfx_leakage.count++; > + } > + } > + else > + { > + printk("%s\n", "Error retrieving EVV voltage value!"); Use DRM_ERROR here. And no need to for %s on this format string. I think you want something like this: DRM_ERROR("Error retrieving EVV voltage value!\n"); > } > } > } else { > @@ -449,19 +452,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr) > if (0 == tonga_get_sclk_for_voltage_evv(hwmgr, > pptable_info->vddc_lookup_table, > virtual_voltage_id, &sclk)) { > - PP_ASSERT_WITH_CODE(0 == atomctrl_get_voltage_evv_on_sclk > + if (0 == atomctrl_get_voltage_evv_on_sclk > (hwmgr, VOLTAGE_TYPE_VDDC, sclk, > - virtual_voltage_id, &vddc), > - "Error retrieving EVV voltage value!", continue); > - > - /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */ > - PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC value!", return -1); > - > - /* the voltage should not be zero nor equal to leakage ID */ > - if (vddc != 0 && vddc != virtual_voltage_id) { > - data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc; > - data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id; > - data->vddc_leakage.count++; > + virtual_voltage_id, &vddc)) { > + /* need to make sure vddc is less than 2v or else, it could burn the ASIC. */ > + PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC value!", return -1); > + > + /* the voltage should not be zero nor equal to leakage ID */ > + if (vddc != 0 && vddc != virtual_voltage_id) { > + data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc; > + data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id; > + data->vddc_leakage.count++; > + } > + } > + else > + { > + printk("%s\n", "Error retrieving EVV voltage value!"); same here. Gustavo _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-15 0:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-14 20:10 [PATCH 0/2] tonga_get_evv_voltage error handling fixes Moritz Kühner 2016-04-14 20:10 ` [PATCH 1/2] drm/amd/powerplay/hwmgr: prevent VDDC from exceeding 2V Moritz Kühner 2016-04-14 20:10 ` [PATCH 2/2] drm/amd/powerplay/hwmgr: don't add invalid voltage Moritz Kühner 2016-04-15 0:34 ` Gustavo Padovan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.