Re: [PATCH xf86-video-amdgpu 17/19] Store device_name as AMDGPUEntRec::master_node

[Michel Dänzer]

On 2018-04-10 11:51 AM, Emil Velikov wrote:
> On 10 April 2018 at 09:29, Michel Dänzer  wrote:
>> On 2018-04-04 04:29 PM, Emil Velikov wrote:
>>> From: Emil Velikov 
>>>
>>> Rename the variable to reflect what it is. Plus move it out of the dri2
>>> section - it's used in dri2 and dri3.
>>>
>>> Signed-off-by: Emil Velikov 
>>
>> [...]
>>
>>> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
>>> index 4959bd6..e9afe42 100644
>>> --- a/src/amdgpu_probe.c
>>> +++ b/src/amdgpu_probe.c
>>> @@ -178,6 +178,10 @@ static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
>>>   if (pAMDGPUEnt->fd < 0)
>>>   return FALSE;
>>>
>>> + pAMDGPUEnt->master_node = drmGetDeviceNameFromFd2(pAMDGPUEnt->fd);
>>> + if (pAMDGPUEnt->master_node)
>>> +goto error_amdgpu;
>>
>> This should be
>>
>> if (!pAMDGPUEnt->master_node)
>>
>> shouldn't it?
>>
>>
>> ... Which raises the question: How did you test these patches? :)
>>
> I mentioned it in the cover letter, but seems to have dropped it -
> they are untested.
> There's a r600 card close-by I could test with, but no amdgpu one :-\

Okay. I can probably test this series, but in general it's preferable
for patches to be tested before sending them out for review.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 17/19] Store device_name as AMDGPUEntRec::master_node

[Emil Velikov]

On 10 April 2018 at 09:29, Michel Dänzer  wrote:
> On 2018-04-04 04:29 PM, Emil Velikov wrote:
>> From: Emil Velikov 
>>
>> Rename the variable to reflect what it is. Plus move it out of the dri2
>> section - it's used in dri2 and dri3.
>>
>> Signed-off-by: Emil Velikov 
>
> [...]
>
>> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
>> index 4959bd6..e9afe42 100644
>> --- a/src/amdgpu_probe.c
>> +++ b/src/amdgpu_probe.c
>> @@ -178,6 +178,10 @@ static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
>>   if (pAMDGPUEnt->fd < 0)
>>   return FALSE;
>>
>> + pAMDGPUEnt->master_node = drmGetDeviceNameFromFd2(pAMDGPUEnt->fd);
>> + if (pAMDGPUEnt->master_node)
>> +goto error_amdgpu;
>
> This should be
>
> if (!pAMDGPUEnt->master_node)
>
> shouldn't it?
>
>
> ... Which raises the question: How did you test these patches? :)
>
I mentioned it in the cover letter, but seems to have dropped it -
they are untested.
There's a r600 card close-by I could test with, but no amdgpu one :-\

-Emil
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 17/19] Store device_name as AMDGPUEntRec::master_node

[Michel Dänzer]

On 2018-04-04 04:29 PM, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Rename the variable to reflect what it is. Plus move it out of the dri2
> section - it's used in dri2 and dri3.
> 
> Signed-off-by: Emil Velikov 

[...]

> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index 4959bd6..e9afe42 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -178,6 +178,10 @@ static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
>   if (pAMDGPUEnt->fd < 0)
>   return FALSE;
>  
> + pAMDGPUEnt->master_node = drmGetDeviceNameFromFd2(pAMDGPUEnt->fd);
> + if (pAMDGPUEnt->master_node)
> +goto error_amdgpu;

This should be

if (!pAMDGPUEnt->master_node)

shouldn't it?


... Which raises the question: How did you test these patches? :)


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 17/19] Store device_name as AMDGPUEntRec::master_node

[Emil Velikov]

From: Emil Velikov 

Rename the variable to reflect what it is. Plus move it out of the dri2
section - it's used in dri2 and dri3.

Signed-off-by: Emil Velikov 
---
 src/amdgpu_dri2.c  | 8 +---
 src/amdgpu_dri2.h  | 1 -
 src/amdgpu_dri3.c  | 3 +--
 src/amdgpu_kms.c   | 1 +
 src/amdgpu_probe.c | 5 +
 src/amdgpu_probe.h | 1 +
 6 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
index 62964ab..082520a 100644
--- a/src/amdgpu_dri2.c
+++ b/src/amdgpu_dri2.c
@@ -1282,11 +1282,9 @@ Bool amdgpu_dri2_screen_init(ScreenPtr pScreen)
if (!info->dri2.available)
return FALSE;
 
-   info->dri2.device_name = drmGetDeviceNameFromFd(pAMDGPUEnt->fd);
-
dri2_info.driverName = SI_DRIVER_NAME;
dri2_info.fd = pAMDGPUEnt->fd;
-   dri2_info.deviceName = info->dri2.device_name;
+   dri2_info.deviceName = pAMDGPUEnt->master_node;
 
if (info->drmmode.count_crtcs > 2) {
uint64_t cap_value;
@@ -1340,15 +1338,11 @@ Bool amdgpu_dri2_screen_init(ScreenPtr pScreen)
 
 void amdgpu_dri2_close_screen(ScreenPtr pScreen)
 {
-   ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-   AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
-
if (--DRI2InfoCnt == 0)
DeleteCallback(,
   amdgpu_dri2_client_state_changed, 0);
 
DRI2CloseScreen(pScreen);
-   drmFree(info->dri2.device_name);
 }
 
 #endif /* DRI2 */
diff --git a/src/amdgpu_dri2.h b/src/amdgpu_dri2.h
index a345e6b..a9411b2 100644
--- a/src/amdgpu_dri2.h
+++ b/src/amdgpu_dri2.h
@@ -32,7 +32,6 @@
 struct amdgpu_dri2 {
Bool available;
Bool enabled;
-   char *device_name;
 };
 
 #ifdef DRI2
diff --git a/src/amdgpu_dri3.c b/src/amdgpu_dri3.c
index 87e3d85..2c06b74 100644
--- a/src/amdgpu_dri3.c
+++ b/src/amdgpu_dri3.c
@@ -44,11 +44,10 @@ static int open_master_node(ScreenPtr screen, int *out)
 {
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
-   AMDGPUInfoPtr info = AMDGPUPTR(scrn);
drm_magic_t magic;
int fd;
 
-   fd = open(info->dri2.device_name, O_RDWR | O_CLOEXEC);
+   fd = open(pAMDGPUEnt->master_node, O_RDWR | O_CLOEXEC);
if (fd < 0)
return BadAlloc;
 
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index c70cfe5..d27f71d 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -147,6 +147,7 @@ static void AMDGPUFreeRec(ScrnInfoPtr pScrn)
pAMDGPUEnt->fd_ref--;
if (!pAMDGPUEnt->fd_ref) {
amdgpu_device_deinitialize(pAMDGPUEnt->pDev);
+   free(pAMDGPUEnt->master_node);
amdgpu_kernel_close_fd(pAMDGPUEnt);
free(pPriv->ptr);
pPriv->ptr = NULL;
diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 4959bd6..e9afe42 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -178,6 +178,10 @@ static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
if (pAMDGPUEnt->fd < 0)
return FALSE;
 
+   pAMDGPUEnt->master_node = drmGetDeviceNameFromFd2(pAMDGPUEnt->fd);
+   if (pAMDGPUEnt->master_node)
+  goto error_amdgpu;
+
if (amdgpu_device_initialize(pAMDGPUEnt->fd,
 _version,
 _version,
@@ -190,6 +194,7 @@ static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
return TRUE;
 
 error_amdgpu:
+   free(pAMDGPUEnt->master_node);
amdgpu_kernel_close_fd(pAMDGPUEnt);
return FALSE;
 }
diff --git a/src/amdgpu_probe.h b/src/amdgpu_probe.h
index 5f61aab..94f204f 100644
--- a/src/amdgpu_probe.h
+++ b/src/amdgpu_probe.h
@@ -62,6 +62,7 @@ typedef struct {
 
int fd; /* for sharing across zaphod heads   */
int fd_ref;
+   char *master_node;
unsigned long fd_wakeup_registered; /* server generation for which 
fd has been registered for wakeup handling */
int fd_wakeup_ref;
unsigned int assigned_crtcs;
-- 
2.16.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx