[PATCH] drm/amdgpu: fix unbalanced parentheses in a #if0 region

2024-07-17 Thread Masatake YAMATO 
Signed-off-by: Masatake YAMATO 
---
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
index 06d787385ad4..27188dadfbcf 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
@@ -741,12 +741,12 @@ static bool vce_v4_0_check_soft_reset(void *handle)
 */
mutex_lock(&adev->grbm_idx_mutex);
WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0);
-   if (RREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_STATUS) & 
AMDGPU_VCE_STATUS_BUSY_MASK) {
+   if (RREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_STATUS) & 
AMDGPU_VCE_STATUS_BUSY_MASK)) {
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, 
SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, 
SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
}
WREG32_FIELD(GRBM_GFX_INDEX, INSTANCE_INDEX, 0x10);
-   if (RREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_STATUS) & 
AMDGPU_VCE_STATUS_BUSY_MASK) {
+   if (RREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_STATUS) & 
AMDGPU_VCE_STATUS_BUSY_MASK)) {
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, 
SRBM_SOFT_RESET, SOFT_RESET_VCE0, 1);
srbm_soft_reset = REG_SET_FIELD(srbm_soft_reset, 
SRBM_SOFT_RESET, SOFT_RESET_VCE1, 1);
}
@@ -936,16 +936,16 @@ static int vce_v4_0_set_clockgating_state(void *handle,
 
if (enable) {
/* initialize VCE_CLOCK_GATING_A: Clock ON/OFF delay */
-   uint32_t data = RREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_CLOCK_GATING_A);
+   uint32_t data = RREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_CLOCK_GATING_A));
data &= ~(0xf | 0xff0);
data |= ((0x0 << 0) | (0x04 << 4));
-   WREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_CLOCK_GATING_A, 
data);
+   WREG32(SOC15_REG_OFFSET(VCE, 0, mmVCE_CLOCK_GATING_A), 
data);
 
/* initialize VCE_UENC_CLOCK_GATING: Clock ON/OFF delay 
*/
-   data = RREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_UENC_CLOCK_GATING);
+   data = RREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_UENC_CLOCK_GATING));
data &= ~(0xf | 0xff0);
data |= ((0x0 << 0) | (0x04 << 4));
-   WREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_UENC_CLOCK_GATING, data);
+   WREG32(SOC15_REG_OFFSET(VCE, 0, 
mmVCE_UENC_CLOCK_GATING), data);
}
 
vce_v4_0_set_vce_sw_clock_gating(adev, enable);
-- 
2.45.2

[PATCH] drm/amdgpu: fix the ioctl direction for DRM_IOCTL_AMDGPU_GEM_VA

2024-08-06 Thread Masatake YAMATO 
Though drmCommandWriteRead() is used in libdrm [1],
DRM_IOCTL_AMDGPU_GEM_VA uses DRM_IOW for encoding.

[1] 
https://gitlab.freedesktop.org/mesa/drm/-/blob/main/amdgpu/amdgpu_bo.c?ref_type=heads

Signed-off-by: Masatake YAMATO 
---
 include/uapi/drm/amdgpu_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index efe5de6ce208..7325390798bc 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -63,7 +63,7 @@ extern "C" {
 #define DRM_IOCTL_AMDGPU_INFO  DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_INFO, struct drm_amdgpu_info)
 #define DRM_IOCTL_AMDGPU_GEM_METADATA  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_METADATA, struct drm_amdgpu_gem_metadata)
 #define DRM_IOCTL_AMDGPU_GEM_WAIT_IDLE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_WAIT_IDLE, union drm_amdgpu_gem_wait_idle)
-#define DRM_IOCTL_AMDGPU_GEM_VADRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_VA, struct drm_amdgpu_gem_va)
+#define DRM_IOCTL_AMDGPU_GEM_VADRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_VA, struct drm_amdgpu_gem_va)
 #define DRM_IOCTL_AMDGPU_WAIT_CS   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs)
 #define DRM_IOCTL_AMDGPU_GEM_OPDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
 #define DRM_IOCTL_AMDGPU_GEM_USERPTR   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
-- 
2.45.2

Re: [PATCH] drm/amdgpu: fix the ioctl direction for DRM_IOCTL_AMDGPU_GEM_VA

2024-08-07 Thread Masatake YAMATO 
Hi,

> 
> Hi Yamato-san,
> 
> 
> On 2024-08-06 04:43, Masatake YAMATO wrote:
>> Though drmCommandWriteRead() is used in libdrm [1],
>> DRM_IOCTL_AMDGPU_GEM_VA uses DRM_IOW for encoding.
>> 
>> [1] 
>> https://gitlab.freedesktop.org/mesa/drm/-/blob/main/amdgpu/amdgpu_bo.c?ref_type=heads
> 
> AFAICT libdrm doesn't read from the struct drm_amdgpu_gem_va it passes to the 
> ioctl after the latter returns though.
> 
> Also, while amdgpu_gem_va_ioctl (the ioctl implementation in the kernel) 
> modifies the struct:
> 
>   args->va_address &= AMDGPU_GMC_HOLE_MASK;
> 
> that looks like an implementation detail which could be avoided if necessary, 
> and shouldn't be propagated back to user space.
> 
> 
> I'd suggest fixing libdrm to use drmCommandWrite instead.

Thank you.

After reading this mail, I inspected drm_ioctl.c.
I was surprised that drm_ioctl() uses only the offset from DRM_COMMAND_BASE 
when choosing
the real handler for a cmd. Therefore we can switch from drmCommandWriteRead to 
drmCommandWrite
without breaking applications using the library. 

Masatake YAMATO

> 
> -- 
> Earthling Michel Dänzer   \GNOME / Xwayland / Mesa developer
> https://redhat.com \   Libre software enthusiast
>