* [PATCH libdrm] xf86drm.c: Use integer logarithm.
@ 2020-10-26 13:11 Paul Gofman
2020-10-27 23:24 ` Dave Airlie
2020-10-28 8:18 ` Pekka Paalanen
0 siblings, 2 replies; 6+ messages in thread
From: Paul Gofman @ 2020-10-26 13:11 UTC (permalink / raw)
To: dri-devel; +Cc: Paul Gofman
log() is affected by FP control word and can provide inaccurate result.
Fixes Killer Instinct under Wine not being able to find AMD vulkan
device.
Signed-off-by: Paul Gofman <pgofman@codeweavers.com>
---
With the rounding mode the application sets (unsigned int)log2(4) is 1.
The log2_int() implemetation is copied from radeon/radeon_surface.c.
xf86drm.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/xf86drm.c b/xf86drm.c
index 50a6f092..dbb7c14b 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -124,6 +124,22 @@ static drmServerInfoPtr drm_server_info;
static bool drmNodeIsDRM(int maj, int min);
static char *drmGetMinorNameForFD(int fd, int type);
+static unsigned log2_int(unsigned x)
+{
+ unsigned l;
+
+ if (x < 2) {
+ return 0;
+ }
+ for (l = 2; ; l++) {
+ if ((unsigned)(1 << l) > x) {
+ return l - 1;
+ }
+ }
+ return 0;
+}
+
+
drm_public void drmSetServerInfo(drmServerInfoPtr info)
{
drm_server_info = info;
@@ -4001,7 +4017,7 @@ static void drmFoldDuplicatedDevices(drmDevicePtr local_devices[], int count)
for (j = i + 1; j < count; j++) {
if (drmDevicesEqual(local_devices[i], local_devices[j])) {
local_devices[i]->available_nodes |= local_devices[j]->available_nodes;
- node_type = log2(local_devices[j]->available_nodes);
+ node_type = log2_int(local_devices[j]->available_nodes);
memcpy(local_devices[i]->nodes[node_type],
local_devices[j]->nodes[node_type], drmGetMaxNodeName());
drmFreeDevice(&local_devices[j]);
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
2020-10-26 13:11 [PATCH libdrm] xf86drm.c: Use integer logarithm Paul Gofman
@ 2020-10-27 23:24 ` Dave Airlie
2020-10-28 8:18 ` Pekka Paalanen
1 sibling, 0 replies; 6+ messages in thread
From: Dave Airlie @ 2020-10-27 23:24 UTC (permalink / raw)
To: Paul Gofman; +Cc: dri-devel
I've applied this to libdrm master.
Thanks,
Dave.
On Mon, 26 Oct 2020 at 23:27, Paul Gofman <pgofman@codeweavers.com> wrote:
>
> log() is affected by FP control word and can provide inaccurate result.
>
> Fixes Killer Instinct under Wine not being able to find AMD vulkan
> device.
>
> Signed-off-by: Paul Gofman <pgofman@codeweavers.com>
> ---
> With the rounding mode the application sets (unsigned int)log2(4) is 1.
> The log2_int() implemetation is copied from radeon/radeon_surface.c.
>
> xf86drm.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 50a6f092..dbb7c14b 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -124,6 +124,22 @@ static drmServerInfoPtr drm_server_info;
> static bool drmNodeIsDRM(int maj, int min);
> static char *drmGetMinorNameForFD(int fd, int type);
>
> +static unsigned log2_int(unsigned x)
> +{
> + unsigned l;
> +
> + if (x < 2) {
> + return 0;
> + }
> + for (l = 2; ; l++) {
> + if ((unsigned)(1 << l) > x) {
> + return l - 1;
> + }
> + }
> + return 0;
> +}
> +
> +
> drm_public void drmSetServerInfo(drmServerInfoPtr info)
> {
> drm_server_info = info;
> @@ -4001,7 +4017,7 @@ static void drmFoldDuplicatedDevices(drmDevicePtr local_devices[], int count)
> for (j = i + 1; j < count; j++) {
> if (drmDevicesEqual(local_devices[i], local_devices[j])) {
> local_devices[i]->available_nodes |= local_devices[j]->available_nodes;
> - node_type = log2(local_devices[j]->available_nodes);
> + node_type = log2_int(local_devices[j]->available_nodes);
> memcpy(local_devices[i]->nodes[node_type],
> local_devices[j]->nodes[node_type], drmGetMaxNodeName());
> drmFreeDevice(&local_devices[j]);
> --
> 2.26.2
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
2020-10-26 13:11 [PATCH libdrm] xf86drm.c: Use integer logarithm Paul Gofman
2020-10-27 23:24 ` Dave Airlie
@ 2020-10-28 8:18 ` Pekka Paalanen
2020-10-28 10:09 ` Paul Gofman
1 sibling, 1 reply; 6+ messages in thread
From: Pekka Paalanen @ 2020-10-28 8:18 UTC (permalink / raw)
To: Paul Gofman; +Cc: dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2121 bytes --]
On Mon, 26 Oct 2020 16:11:20 +0300
Paul Gofman <pgofman@codeweavers.com> wrote:
> log() is affected by FP control word and can provide inaccurate result.
>
> Fixes Killer Instinct under Wine not being able to find AMD vulkan
> device.
>
> Signed-off-by: Paul Gofman <pgofman@codeweavers.com>
> ---
> With the rounding mode the application sets (unsigned int)log2(4) is 1.
> The log2_int() implemetation is copied from radeon/radeon_surface.c.
>
> xf86drm.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 50a6f092..dbb7c14b 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -124,6 +124,22 @@ static drmServerInfoPtr drm_server_info;
> static bool drmNodeIsDRM(int maj, int min);
> static char *drmGetMinorNameForFD(int fd, int type);
>
> +static unsigned log2_int(unsigned x)
> +{
> + unsigned l;
> +
> + if (x < 2) {
> + return 0;
> + }
> + for (l = 2; ; l++) {
> + if ((unsigned)(1 << l) > x) {
Hi,
wouldn't this loop fail to end when x >= 0x80000000?
Sure, such value probably cannot occur where this is currently used,
but it seems like a landmine for the next developer to step on.
Thanks,
pq
> + return l - 1;
> + }
> + }
> + return 0;
> +}
> +
> +
> drm_public void drmSetServerInfo(drmServerInfoPtr info)
> {
> drm_server_info = info;
> @@ -4001,7 +4017,7 @@ static void drmFoldDuplicatedDevices(drmDevicePtr local_devices[], int count)
> for (j = i + 1; j < count; j++) {
> if (drmDevicesEqual(local_devices[i], local_devices[j])) {
> local_devices[i]->available_nodes |= local_devices[j]->available_nodes;
> - node_type = log2(local_devices[j]->available_nodes);
> + node_type = log2_int(local_devices[j]->available_nodes);
> memcpy(local_devices[i]->nodes[node_type],
> local_devices[j]->nodes[node_type], drmGetMaxNodeName());
> drmFreeDevice(&local_devices[j]);
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
2020-10-28 8:18 ` Pekka Paalanen
@ 2020-10-28 10:09 ` Paul Gofman
2020-10-28 10:30 ` Michel Dänzer
0 siblings, 1 reply; 6+ messages in thread
From: Paul Gofman @ 2020-10-28 10:09 UTC (permalink / raw)
To: Pekka Paalanen; +Cc: dri-devel
On 10/28/20 11:18, Pekka Paalanen wrote:
>
>>
>> +static unsigned log2_int(unsigned x)
>> +{
>> + unsigned l;
>> +
>> + if (x < 2) {
>> + return 0;
>> + }
>> + for (l = 2; ; l++) {
>> + if ((unsigned)(1 << l) > x) {
> Hi,
>
> wouldn't this loop fail to end when x >= 0x80000000?
>
> Sure, such value probably cannot occur where this is currently used,
> but it seems like a landmine for the next developer to step on.
>
Indeed, thanks. I've sent the patches for consideration which avoid
function duplication and potentially infinite loop.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
2020-10-28 10:09 ` Paul Gofman
@ 2020-10-28 10:30 ` Michel Dänzer
2020-10-28 10:36 ` Paul Gofman
0 siblings, 1 reply; 6+ messages in thread
From: Michel Dänzer @ 2020-10-28 10:30 UTC (permalink / raw)
To: Paul Gofman, Pekka Paalanen; +Cc: dri-devel
On 2020-10-28 11:09 a.m., Paul Gofman wrote:
> On 10/28/20 11:18, Pekka Paalanen wrote:
>>
>>>
>>> +static unsigned log2_int(unsigned x)
>>> +{
>>> + unsigned l;
>>> +
>>> + if (x < 2) {
>>> + return 0;
>>> + }
>>> + for (l = 2; ; l++) {
>>> + if ((unsigned)(1 << l) > x) {
>> Hi,
>>
>> wouldn't this loop fail to end when x >= 0x80000000?
>>
>> Sure, such value probably cannot occur where this is currently used,
>> but it seems like a landmine for the next developer to step on.
>>
> Indeed, thanks. I've sent the patches for consideration which avoid
> function duplication and potentially infinite loop.
libdrm uses GitLab merge requests now:
https://gitlab.freedesktop.org/mesa/drm/-/merge_requests
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
2020-10-28 10:30 ` Michel Dänzer
@ 2020-10-28 10:36 ` Paul Gofman
0 siblings, 0 replies; 6+ messages in thread
From: Paul Gofman @ 2020-10-28 10:36 UTC (permalink / raw)
To: Michel Dänzer, Pekka Paalanen; +Cc: dri-devel
On 10/28/20 13:30, Michel Dänzer wrote:
> On 2020-10-28 11:09 a.m., Paul Gofman wrote:
>> On 10/28/20 11:18, Pekka Paalanen wrote:
>>>
>>>> +static unsigned log2_int(unsigned x)
>>>> +{
>>>> + unsigned l;
>>>> +
>>>> + if (x < 2) {
>>>> + return 0;
>>>> + }
>>>> + for (l = 2; ; l++) {
>>>> + if ((unsigned)(1 << l) > x) {
>>> Hi,
>>>
>>> wouldn't this loop fail to end when x >= 0x80000000?
>>>
>>> Sure, such value probably cannot occur where this is currently used,
>>> but it seems like a landmine for the next developer to step on.
>>>
>> Indeed, thanks. I've sent the patches for consideration which avoid
>> function duplication and potentially infinite loop.
>
> libdrm uses GitLab merge requests now:
> https://gitlab.freedesktop.org/mesa/drm/-/merge_requests
>
>
I see, thanks. I was following the instructions in CONTRIBUTING.rst.
Do you think I should proceed with merge request for these patches or
those already sent could be considered this way?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-28 10:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-26 13:11 [PATCH libdrm] xf86drm.c: Use integer logarithm Paul Gofman
2020-10-27 23:24 ` Dave Airlie
2020-10-28 8:18 ` Pekka Paalanen
2020-10-28 10:09 ` Paul Gofman
2020-10-28 10:30 ` Michel Dänzer
2020-10-28 10:36 ` Paul Gofman
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.