Re: [PATCH 11/11] drm: remove unused and redundant callbacks

[Daniel Vetter]

On Thu, Jun 22, 2017 at 12:34:36AM +0800, kbuild test robot wrote:
> Hi Peter,
> 
> [auto build test ERROR on drm/drm-next]
> [also build test ERROR on next-20170621]
> [cannot apply to v4.12-rc6]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441
> base:   git://people.freedesktop.org/~airlied/linux.git drm-next
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget 
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field 
> >> 'gamma_set' specified in initializer
>  .gamma_set = armada_drm_crtc_gamma_set,

Looks like you've missed at least the armada driver in your conversion?
-Daniel

>  ^
> >> drivers/gpu//drm/armada/armada_fbdev.c:121:15: error: initialization from 
> >> incompatible pointer type [-Werror=incompatible-pointer-types]
>  .gamma_set = armada_drm_crtc_gamma_set,
>   ^
>drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization 
> for 'armada_fb_helper_funcs.fb_probe')
> >> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field 
> >> 'gamma_get' specified in initializer
>  .gamma_get = armada_drm_crtc_gamma_get,
>  ^
>drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from 
> incompatible pointer type [-Werror=incompatible-pointer-types]
>  .gamma_get = armada_drm_crtc_gamma_get,
>   ^
>drivers/gpu//drm/armada/armada_fbdev.c:122:15: note: (near initialization 
> for 'armada_fb_helper_funcs.initial_config')
>cc1: some warnings being treated as errors
> 
> vim +/gamma_set +121 drivers/gpu//drm/armada/armada_fbdev.c
> 
> 96f60e37 Russell King   2012-08-15  115   ret = 1;
> 96f60e37 Russell King   2012-08-15  116   }
> 96f60e37 Russell King   2012-08-15  117   return ret;
> 96f60e37 Russell King   2012-08-15  118  }
> 96f60e37 Russell King   2012-08-15  119  
> 3a493879 Thierry Reding 2014-06-27  120  static const struct 
> drm_fb_helper_funcs armada_fb_helper_funcs = {
> 96f60e37 Russell King   2012-08-15 @121   .gamma_set  = 
> armada_drm_crtc_gamma_set,
> 96f60e37 Russell King   2012-08-15 @122   .gamma_get  = 
> armada_drm_crtc_gamma_get,
> 96f60e37 Russell King   2012-08-15  123   .fb_probe   = 
> armada_fb_probe,
> 96f60e37 Russell King   2012-08-15  124  };
> 96f60e37 Russell King   2012-08-15  125  
> 
> :: The code at line 121 was first introduced by commit
> :: 96f60e37dc66091bde8d5de136ff6fda09f2d799 DRM: Armada: Add Armada DRM 
> driver
> 
> :: TO: Russell King 
> :: CC: Russell King 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation


> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Wed, Jun 21, 2017 at 11:40:52AM +0200, Peter Rosin wrote:
> On 2017-06-21 09:38, Daniel Vetter wrote:
> > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >> totally obsolete.
> >>
> >> I think the gamma_store can end up invalid on error. But the way I read
> >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> >> this pesky legacy fbdev stuff be any better?
> >>
> >> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> >> it saves it to the gamma_store which should already be up to date with what
> >> .gamma_get would return and is thus a nop. So, zap it.
> > 
> > Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> > think.
> 
> Then 3 patches would be needed, first some hybrid thing that does it the
> old way, but also stores the lut in .gamma_store, then the split-out that
> removes drm_fb_helper_save_lut_atomic, then whatever is needed to get
> to the desired code. I can certainly do that, but do you want me to?

Explain that in the commit message and it's fine.

> > It's a pre-existing bug, but should we also try to restore the fbdev lut
> > in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> > but might be relevant for your use-case. Just try to run both an fbdev
> > application and some kms-native thing, and then SIGKILL the native kms
> > app.
> > 
> > But since pre-existing not really required, and probably too much effort.
> 
> Good thing too, because I don't really know my way around this code...

Btw I cc'ed you on one of my patches in the fbdev locking series, we might
need to do the same legacy vs. atomic split for the new lut code as I did
for dpms. The rule with atomic is that you can't do multiple commits under
drm_modeset_lock_all, you either have to do one overall atomic commit
(preferred) or drop&reacquire locks again. This matters for LUT since
you're updating the LUT on all CRTCs, which when using the gamma_set
atomic helper would be multiple commits :-/

Using the dpms patch as template it shouldn't be too hard to address that
for your patch here too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu] Increase reference count of FB assigned to drmmode_crtc->flip_pending

[Michel Dänzer]

From: Michel Dänzer 

Otherwise, it could happen that we destroy the FB before the flip
completes, resulting in use-after-free and most likely a crash.

Signed-off-by: Michel Dänzer 
---
 src/amdgpu_kms.c  | 8 
 src/drmmode_display.c | 8 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 784f7388a..143294a9f 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -722,8 +722,8 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
return;
}
 
-   drmmode_crtc->flip_pending =
-   amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+   drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
+
amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
if (!drmmode_crtc->flip_pending) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
   "Failed to get FB for PRIME flip.\n");
@@ -1011,8 +1011,8 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info,
return;
}
 
-   drmmode_crtc->flip_pending =
-   amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+   drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
+
amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
if (!drmmode_crtc->flip_pending) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
   "Failed to get FB for scanout flip.\n");
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 0d900418a..ce46f7ba6 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2196,8 +2196,11 @@ void
 drmmode_clear_pending_flip(xf86CrtcPtr crtc)
 {
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+   ScrnInfoPtr scrn = crtc->scrn;
+   AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 
-   drmmode_crtc->flip_pending = NULL;
+   drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->flip_pending,
+NULL);
 
if (!crtc->enabled ||
(drmmode_crtc->pending_dpms_mode != DPMSModeOn &&
@@ -2835,7 +2838,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr 
client,
goto flip_error;
}
 
-   drmmode_crtc->flip_pending = fb;
+   drmmode_fb_reference(pAMDGPUEnt->fd, 
&drmmode_crtc->flip_pending,
+fb);
drm_queue_seq = 0;
}
 
-- 
2.11.0

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

[PATCH xf86-video-ati] Only call drmmode_scanout_free for non-GPU screens in LeaveVT

[Michel Dänzer]

From: Michel Dänzer 

Destroying the scanout buffers of GPU screens resulted in a crash when
switching back to the Xorg VT.

Signed-off-by: Michel Dänzer 
---
 src/radeon_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index c4bdfcfac..5637e7f8a 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -2478,7 +2478,8 @@ void RADEONLeaveVT_KMS(VT_FUNC_ARGS_DECL)
 radeon_drop_drm_master(pScrn);
 
 xf86RotateFreeShadow(pScrn);
-drmmode_scanout_free(pScrn);
+if (!pScrn->is_gpu)
+   drmmode_scanout_free(pScrn);
 
 xf86_hide_cursors (pScrn);
 info->accel_state->XInited3D = FALSE;
-- 
2.11.0

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

[PATCH v2] drm/amdgpu: export gpu always on cu bitmap

[Flora Cui]

v2: keep cu_ao_mask for backward compatibility.

Change-Id: I056d8af23340d46e5140bd10cc38dfb887cc78ab
Signed-off-by: Flora Cui 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 4 +++-
 include/uapi/drm/amdgpu_drm.h   | 3 +++
 8 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6b7d2a1..6b9a91c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1029,12 +1029,15 @@ struct amdgpu_gfx_config {
 };
 
 struct amdgpu_cu_info {
-   uint32_t number; /* total active CU number */
-   uint32_t ao_cu_mask;
uint32_t max_waves_per_simd;
uint32_t wave_front_size;
uint32_t max_scratch_slots_per_cu;
uint32_t lds_size;
+
+   /* total active CU number */
+   uint32_t number;
+   uint32_t ao_cu_mask;
+   uint32_t ao_cu_bitmap[4][4];
uint32_t bitmap[4][4];
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0424711..5a1d794 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -67,9 +67,10 @@
  * - 3.15.0 - Export more gpu info for gfx9
  * - 3.16.0 - Add reserved vmid support
  * - 3.17.0 - Add AMDGPU_NUM_VRAM_CPU_PAGE_FAULTS.
+ * - 3.18.0 - Export gpu always on cu bitmap
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   17
+#define KMS_DRIVER_MINOR   18
 #define KMS_DRIVER_PATCHLEVEL  0
 
 int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f68ced6..eff2e11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -591,8 +591,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE;
 
dev_info.cu_active_number = adev->gfx.cu_info.number;
-   dev_info.cu_ao_mask = adev->gfx.cu_info.ao_cu_mask;
+   dev_info.cu_ao_mask = 0;
dev_info.ce_ram_size = adev->gfx.ce_ram_size;
+   memcpy(&dev_info.cu_ao_bitmap[0], 
&adev->gfx.cu_info.ao_cu_bitmap[0],
+  sizeof(adev->gfx.cu_info.ao_cu_bitmap));
memcpy(&dev_info.cu_bitmap[0], &adev->gfx.cu_info.bitmap[0],
   sizeof(adev->gfx.cu_info.bitmap));
dev_info.vram_type = adev->mc.vram_type;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 7b0b3cf..5173ca1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -3535,7 +3535,9 @@ static void gfx_v6_0_get_cu_info(struct amdgpu_device 
*adev)
mask <<= 1;
}
active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
}
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index fb0a94c..8c4dd7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -5427,7 +5427,9 @@ static void gfx_v7_0_get_cu_info(struct amdgpu_device 
*adev)
mask <<= 1;
}
active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
}
}
gfx_v7_0_select_se_sh(adev, 0x, 0x, 0x);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 1a75ab1..9edb509 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -7080,7 +7080,9 @@ static void gfx_v8_0_get_cu_info(struct amdgpu_device 
*adev)
mask <<= 1;
}
active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
   

Re: [PATCH] drm/amdgpu: optimize out a spin lock (v2)

[Michel Dänzer]

On 22/06/17 11:45 AM, Alex Xie wrote:
> Use atomic instead of spin lock.
> v2: Adjust commit message
> 
> Signed-off-by: Alex Xie 

The shortlog should be more specific, e.g. something like "drm/amdgpu:
Drop spinlock from mm_stats".

It's important for the Git commit shortlog to be as specific as possible
because in many cases only the shortlogs of commits are visible.


I'll leave it to others to judge whether the conversion from spinlock to
atomics is safe / an overall win.


>   /* This returns 0 if the driver is in debt to disallow (optional)
>* buffer moves.
>*/
> - max_bytes = us_to_bytes(adev, adev->mm_stats.accum_us);
> -
> - spin_unlock(&adev->mm_stats.lock);
> + max_bytes = us_to_bytes(adev, accum_us);
>   return max_bytes;
>  }

You can just make this

return us_to_bytes(adev, accum_us);

and remove the max_bytes local.


-- 
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 2/3] drm/amdgpu: change a function to static function

[Michel Dänzer]

On 22/06/17 11:42 AM, Alex Xie wrote:
> The function is called only once inside the .c file.
> 
> Signed-off-by: Alex Xie 

The shortlog should explicitly say "drm/amdgpu: Make
amdgpu_cs_parser_init static". With that, this patch and patch 1 are

Reviewed-by: Michel Dänzer 


-- 
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] drm/amdgpu: optimize out a spin lock (v2)

[Alex Xie]

Use atomic instead of spin lock.
v2: Adjust commit message

Signed-off-by: Alex Xie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 110 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   1 -
 3 files changed, 76 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7caf514..21d318b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1588,9 +1588,8 @@ struct amdgpu_device {
 
/* data for buffer migration throttling */
struct {
-   spinlock_t  lock;
-   s64 last_update_us;
-   s64 accum_us; /* accumulated microseconds */
+   atomic64_t  last_update_us;
+   atomic64_t  accum_us; /* accumulated microseconds */
u32 log2_max_MBps;
} mm_stats;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 82131d7..7b6f42e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -225,6 +225,9 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct 
amdgpu_device *adev)
s64 time_us, increment_us;
u64 max_bytes;
u64 free_vram, total_vram, used_vram;
+   s64 old_update_us, head_time_us;
+   s64 accum_us;
+   s64 old_accum_us, head_accum_us;
 
/* Allow a maximum of 200 accumulated ms. This is basically per-IB
 * throttling.
@@ -242,47 +245,83 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct 
amdgpu_device *adev)
used_vram = atomic64_read(&adev->vram_usage);
free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
 
-   spin_lock(&adev->mm_stats.lock);
-
/* Increase the amount of accumulated us. */
-   time_us = ktime_to_us(ktime_get());
-   increment_us = time_us - adev->mm_stats.last_update_us;
-   adev->mm_stats.last_update_us = time_us;
-   adev->mm_stats.accum_us = min(adev->mm_stats.accum_us + increment_us,
-  us_upper_bound);
-
-   /* This prevents the short period of low performance when the VRAM
-* usage is low and the driver is in debt or doesn't have enough
-* accumulated us to fill VRAM quickly.
-*
-* The situation can occur in these cases:
-* - a lot of VRAM is freed by userspace
-* - the presence of a big buffer causes a lot of evictions
-*   (solution: split buffers into smaller ones)
-*
-* If 128 MB or 1/8th of VRAM is free, start filling it now by setting
-* accum_us to a positive number.
-*/
-   if (free_vram >= 128 * 1024 * 1024 || free_vram >= total_vram / 8) {
-   s64 min_us;
-
-   /* Be more aggresive on dGPUs. Try to fill a portion of free
-* VRAM now.
-*/
-   if (!(adev->flags & AMD_IS_APU))
-   min_us = bytes_to_us(adev, free_vram / 4);
+   old_update_us = atomic64_read(&adev->mm_stats.last_update_us);
+   for (;;) {
+   time_us = ktime_to_us(ktime_get());
+   head_time_us = atomic64_cmpxchg(&adev->mm_stats.last_update_us,
+   old_update_us, time_us);
+
+   if (likely(head_time_us == old_update_us))
+   /*
+* No other task modified adev->mm_stats.last_update_us.
+* Update was successful.
+*/
+   break;
else
-   min_us = 0; /* Reset accum_us on APUs. */
+   /* Another task modified the value after we read it.
+* A rare contention happens, let us retry.
+* In most case, one retry can do the job.
+* See function atomic64_add_unless as a similar idea.
+*/
+   old_update_us = head_time_us;
+   }
+   increment_us = time_us - old_update_us;
+
+   old_accum_us = atomic64_read(&adev->mm_stats.accum_us);
+
+   for (;;) {
+   accum_us = min(old_accum_us + increment_us, us_upper_bound);
+
+   /* This prevents the short period of low performance when the
+* VRAM usage is low and the driver is in debt or doesn't have
+* enough accumulated us to fill VRAM quickly.
+*
+* The situation can occur in these cases:
+* - a lot of VRAM is freed by userspace
+* - the presence of a big buffer causes a lot of evictions
+*   (solution: split buffers into smaller ones)
+*
+* If 128 MB o

[PATCH 2/3] drm/amdgpu: change a function to static function

[Alex Xie]

The function is called only once inside the .c file.

Signed-off-by: Alex Xie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6b7d2a1..7caf514 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1929,7 +1929,6 @@ void amdgpu_pci_config_reset(struct amdgpu_device *adev);
 bool amdgpu_need_post(struct amdgpu_device *adev);
 void amdgpu_update_display_priority(struct amdgpu_device *adev);
 
-int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data);
 void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes);
 void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
 bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 94c27fc..82131d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -63,7 +63,7 @@ static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser 
*p,
return 0;
 }
 
-int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
+static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
 {
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
struct amdgpu_vm *vm = &fpriv->vm;
-- 
2.7.4

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

[PATCH 3/3] drm/amdgpu: optimize out a spin lock Use atomic instead of spin lock.

[Alex Xie]

Signed-off-by: Alex Xie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 110 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   1 -
 3 files changed, 76 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7caf514..21d318b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1588,9 +1588,8 @@ struct amdgpu_device {
 
/* data for buffer migration throttling */
struct {
-   spinlock_t  lock;
-   s64 last_update_us;
-   s64 accum_us; /* accumulated microseconds */
+   atomic64_t  last_update_us;
+   atomic64_t  accum_us; /* accumulated microseconds */
u32 log2_max_MBps;
} mm_stats;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 82131d7..7b6f42e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -225,6 +225,9 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct 
amdgpu_device *adev)
s64 time_us, increment_us;
u64 max_bytes;
u64 free_vram, total_vram, used_vram;
+   s64 old_update_us, head_time_us;
+   s64 accum_us;
+   s64 old_accum_us, head_accum_us;
 
/* Allow a maximum of 200 accumulated ms. This is basically per-IB
 * throttling.
@@ -242,47 +245,83 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct 
amdgpu_device *adev)
used_vram = atomic64_read(&adev->vram_usage);
free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
 
-   spin_lock(&adev->mm_stats.lock);
-
/* Increase the amount of accumulated us. */
-   time_us = ktime_to_us(ktime_get());
-   increment_us = time_us - adev->mm_stats.last_update_us;
-   adev->mm_stats.last_update_us = time_us;
-   adev->mm_stats.accum_us = min(adev->mm_stats.accum_us + increment_us,
-  us_upper_bound);
-
-   /* This prevents the short period of low performance when the VRAM
-* usage is low and the driver is in debt or doesn't have enough
-* accumulated us to fill VRAM quickly.
-*
-* The situation can occur in these cases:
-* - a lot of VRAM is freed by userspace
-* - the presence of a big buffer causes a lot of evictions
-*   (solution: split buffers into smaller ones)
-*
-* If 128 MB or 1/8th of VRAM is free, start filling it now by setting
-* accum_us to a positive number.
-*/
-   if (free_vram >= 128 * 1024 * 1024 || free_vram >= total_vram / 8) {
-   s64 min_us;
-
-   /* Be more aggresive on dGPUs. Try to fill a portion of free
-* VRAM now.
-*/
-   if (!(adev->flags & AMD_IS_APU))
-   min_us = bytes_to_us(adev, free_vram / 4);
+   old_update_us = atomic64_read(&adev->mm_stats.last_update_us);
+   for (;;) {
+   time_us = ktime_to_us(ktime_get());
+   head_time_us = atomic64_cmpxchg(&adev->mm_stats.last_update_us,
+   old_update_us, time_us);
+
+   if (likely(head_time_us == old_update_us))
+   /*
+* No other task modified adev->mm_stats.last_update_us.
+* Update was successful.
+*/
+   break;
else
-   min_us = 0; /* Reset accum_us on APUs. */
+   /* Another task modified the value after we read it.
+* A rare contention happens, let us retry.
+* In most case, one retry can do the job.
+* See function atomic64_add_unless as a similar idea.
+*/
+   old_update_us = head_time_us;
+   }
+   increment_us = time_us - old_update_us;
+
+   old_accum_us = atomic64_read(&adev->mm_stats.accum_us);
+
+   for (;;) {
+   accum_us = min(old_accum_us + increment_us, us_upper_bound);
+
+   /* This prevents the short period of low performance when the
+* VRAM usage is low and the driver is in debt or doesn't have
+* enough accumulated us to fill VRAM quickly.
+*
+* The situation can occur in these cases:
+* - a lot of VRAM is freed by userspace
+* - the presence of a big buffer causes a lot of evictions
+*   (solution: split buffers into smaller ones)
+*
+* If 128 MB or 1/8th of VRAM is free, start filling it now by
+  

[PATCH 1/3] drm/amdgpu: fix a typo

[Alex Xie]

Signed-off-by: Alex Xie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 7635f38..94c27fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -494,7 +494,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 &e->user_invalidated) && e->user_pages) {
 
/* We acquired a page array, but somebody
-* invalidated it. Free it an try again
+* invalidated it. Free it and try again
 */
release_pages(e->user_pages,
  e->robj->tbo.ttm->num_pages,
-- 
2.7.4

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

Re: [PATCH 1/4] drm/vc4: Allow vblank_disable_immediate on non-fw-kms.

[Mario Kleiner]

On 06/21/2017 06:19 PM, Eric Anholt wrote:

Mario Kleiner  writes:


With instantaneous high precision vblank timestamping
that updates at leading edge of vblank, the emulated
"hw vblank counter" from vblank timestamping which
increments at leading edge of vblank, and reliable
page flip execution and completion at leading edge
of vblank, we should meet the requirements for fast
vblank irq disable/enable.

Testing against rpi-4.12-rc5 Linux kernel with timing
measurement equipment indicates this works fine,
so allow immediate vblank disable for power saving.

For debugging in case of unexpected trouble, booting
with kernel cmdline option drm.vblankoffdelay=0
would keep vblank irqs on to approximate old behavior.

Signed-off-by: Mario Kleiner 
Cc: Eric Anholt 


If you can spin this against drm-misc-next instead of the downstream
tree, I can get it applied.



Done, thanks!
-mario
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/4] drm/amdgpu: Allow vblank_disable_immediate.

[Alex Deucher]

On Wed, Jun 21, 2017 at 3:22 AM, Michel Dänzer  wrote:
> On 21/06/17 10:44 AM, Mario Kleiner wrote:
>> With instantaneous high precision vblank timestamping
>> that updates at leading edge of vblank, a cooked hw
>> vblank counter which increments at leading edge of
>> vblank, and reliable page flip execution and completion
>> at leading edge of vblank, we should meet the requirements
>> for fast/immediate vblank irq disable/enable.
>>
>> Testing on Linux-4.12-rc5 + drm-next on a Radeon R9 380
>> Tonga Pro (DCE 10) with timing measurement equipment
>> indicates this works fine, so allow immediate vblank
>> disable for power saving.
>>
>> For debugging in case of unexpected trouble, booting
>> with kernel cmdline option drm.vblankoffdelay=0
>> (or echo 0 > /sys/module/drm/parameters/vblankoffdelay)
>> would keep vblank irqs permanently on to approximate old
>> behavior.
>>
>> Signed-off-by: Mario Kleiner 
>> Cc: Alex Deucher 
>> Cc: Michel Dänzer 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> index 62da6c5..a28f8aa 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> @@ -220,6 +220,10 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>>   int r = 0;
>>
>>   spin_lock_init(&adev->irq.lock);
>> +
>> + /* Disable vblank irqs aggressively for power-saving */
>> + adev->ddev->vblank_disable_immediate = true;
>> +
>>   r = drm_vblank_init(adev->ddev, adev->mode_info.num_crtc);
>>   if (r) {
>>   return r;
>>
>
> Reviewed-by: Michel Dänzer 


Applied patches 2 and 3.

Thanks,

Alex

>
>
> --
> 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
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: fix typo in amdgpu_debugfs_test_ib_init

[Alex Deucher]

On Wed, Jun 21, 2017 at 5:51 PM, Arnd Bergmann  wrote:
> The debugfs interface has calls a function that was evidently
> defined under the wrong name in some configurations:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:64:12: error: 
> 'amdgpu_debugfs_test_ib_ring_init' used but never defined [-Werror]
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3803:12: error: 
> 'amdgpu_debugfs_test_ib_init' defined but not used [-Werror=unused-function]
>
> This fixes the function name.
>
> Fixes: 4f0955fcc052 ("drm/amdgpu: export test ib debugfs interface")
> Signed-off-by: Arnd Bergmann 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2ee7c50c70f9..4a8fc15467cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3800,7 +3800,7 @@ int amdgpu_debugfs_init(struct drm_minor *minor)
> return 0;
>  }
>  #else
> -static int amdgpu_debugfs_test_ib_init(struct amdgpu_device *adev)
> +static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
>  {
> return 0;
>  }
> --
> 2.9.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix typo in amdgpu_debugfs_test_ib_init

[Arnd Bergmann]

The debugfs interface has calls a function that was evidently
defined under the wrong name in some configurations:

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:64:12: error: 
'amdgpu_debugfs_test_ib_ring_init' used but never defined [-Werror]
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3803:12: error: 
'amdgpu_debugfs_test_ib_init' defined but not used [-Werror=unused-function]

This fixes the function name.

Fixes: 4f0955fcc052 ("drm/amdgpu: export test ib debugfs interface")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2ee7c50c70f9..4a8fc15467cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3800,7 +3800,7 @@ int amdgpu_debugfs_init(struct drm_minor *minor)
return 0;
 }
 #else
-static int amdgpu_debugfs_test_ib_init(struct amdgpu_device *adev)
+static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev)
 {
return 0;
 }
-- 
2.9.0

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

Re: [PATCH 1/4] drm/vc4: Allow vblank_disable_immediate on non-fw-kms.

[kbuild test robot]

Hi Mario,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.12-rc6 next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Mario-Kleiner/drm-vc4-Allow-vblank_disable_immediate-on-non-fw-kms/20170622-013723
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/vc4/vc4_kms.c: In function 'vc4_kms_load':
>> drivers/gpu//drm/vc4/vc4_kms.c:220:10: error: 'struct vc4_dev' has no member 
>> named 'firmware_kms'
 if (!vc4->firmware_kms)
 ^~

vim +220 drivers/gpu//drm/vc4/vc4_kms.c

   214  struct vc4_dev *vc4 = to_vc4_dev(dev);
   215  int ret;
   216  
   217  sema_init(&vc4->async_modeset, 1);
   218  
   219  /* Set support for vblank irq fast disable, before 
drm_vblank_init() */
 > 220  if (!vc4->firmware_kms)
   221  dev->vblank_disable_immediate = true;
   222  
   223  ret = drm_vblank_init(dev, dev->mode_config.num_crtc);

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: use kernel is_power_of_2 rather than local version

[Christian König]

Am 21.06.2017 um 18:44 schrieb Alex Deucher:

Use the kernel provided version.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++
  1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6c64551..b6efda1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1032,19 +1032,6 @@ static unsigned int amdgpu_vga_set_decode(void *cookie, 
bool state)
return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
  }
  
-/**

- * amdgpu_check_pot_argument - check that argument is a power of two
- *
- * @arg: value to check
- *
- * Validates that a certain argument is a power of two (all asics).
- * Returns true if argument is valid.
- */
-static bool amdgpu_check_pot_argument(int arg)
-{
-   return (arg & (arg - 1)) == 0;
-}
-
  static void amdgpu_check_block_size(struct amdgpu_device *adev)
  {
/* defines number of bits in page table versus page directory,
@@ -1078,7 +1065,7 @@ static void amdgpu_check_vm_size(struct amdgpu_device 
*adev)
if (amdgpu_vm_size == -1)
return;
  
-	if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {

+   if (!is_power_of_2(amdgpu_vm_size)) {
dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
 amdgpu_vm_size);
goto def_value;
@@ -1119,7 +1106,7 @@ static void amdgpu_check_arguments(struct amdgpu_device 
*adev)
dev_warn(adev->dev, "sched jobs (%d) must be at least 4\n",
 amdgpu_sched_jobs);
amdgpu_sched_jobs = 4;
-   } else if (!amdgpu_check_pot_argument(amdgpu_sched_jobs)){
+   } else if (!is_power_of_2(amdgpu_sched_jobs)){
dev_warn(adev->dev, "sched jobs (%d) must be a power of 2\n",
 amdgpu_sched_jobs);
amdgpu_sched_jobs = roundup_pow_of_two(amdgpu_sched_jobs);
@@ -1139,7 +1126,7 @@ static void amdgpu_check_arguments(struct amdgpu_device 
*adev)
amdgpu_check_block_size(adev);
  
  	if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||

-   !amdgpu_check_pot_argument(amdgpu_vram_page_split))) {
+   !is_power_of_2(amdgpu_vram_page_split))) {
dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
 amdgpu_vram_page_split);
amdgpu_vram_page_split = 1024;



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

[PATCH] drm/amdgpu: use kernel is_power_of_2 rather than local version

[Alex Deucher]

Use the kernel provided version.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6c64551..b6efda1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1032,19 +1032,6 @@ static unsigned int amdgpu_vga_set_decode(void *cookie, 
bool state)
return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
 }
 
-/**
- * amdgpu_check_pot_argument - check that argument is a power of two
- *
- * @arg: value to check
- *
- * Validates that a certain argument is a power of two (all asics).
- * Returns true if argument is valid.
- */
-static bool amdgpu_check_pot_argument(int arg)
-{
-   return (arg & (arg - 1)) == 0;
-}
-
 static void amdgpu_check_block_size(struct amdgpu_device *adev)
 {
/* defines number of bits in page table versus page directory,
@@ -1078,7 +1065,7 @@ static void amdgpu_check_vm_size(struct amdgpu_device 
*adev)
if (amdgpu_vm_size == -1)
return;
 
-   if (!amdgpu_check_pot_argument(amdgpu_vm_size)) {
+   if (!is_power_of_2(amdgpu_vm_size)) {
dev_warn(adev->dev, "VM size (%d) must be a power of 2\n",
 amdgpu_vm_size);
goto def_value;
@@ -1119,7 +1106,7 @@ static void amdgpu_check_arguments(struct amdgpu_device 
*adev)
dev_warn(adev->dev, "sched jobs (%d) must be at least 4\n",
 amdgpu_sched_jobs);
amdgpu_sched_jobs = 4;
-   } else if (!amdgpu_check_pot_argument(amdgpu_sched_jobs)){
+   } else if (!is_power_of_2(amdgpu_sched_jobs)){
dev_warn(adev->dev, "sched jobs (%d) must be a power of 2\n",
 amdgpu_sched_jobs);
amdgpu_sched_jobs = roundup_pow_of_two(amdgpu_sched_jobs);
@@ -1139,7 +1126,7 @@ static void amdgpu_check_arguments(struct amdgpu_device 
*adev)
amdgpu_check_block_size(adev);
 
if (amdgpu_vram_page_split != -1 && (amdgpu_vram_page_split < 16 ||
-   !amdgpu_check_pot_argument(amdgpu_vram_page_split))) {
+   !is_power_of_2(amdgpu_vram_page_split))) {
dev_warn(adev->dev, "invalid VRAM page split (%d)\n",
 amdgpu_vram_page_split);
amdgpu_vram_page_split = 1024;
-- 
2.5.5

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

Re: [PATCH 11/11] drm: remove unused and redundant callbacks

[kbuild test robot]

Hi Peter,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20170621]
[cannot apply to v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field 
>> 'gamma_set' specified in initializer
 .gamma_set = armada_drm_crtc_gamma_set,
 ^
>> drivers/gpu//drm/armada/armada_fbdev.c:121:15: error: initialization from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
 .gamma_set = armada_drm_crtc_gamma_set,
  ^
   drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization 
for 'armada_fb_helper_funcs.fb_probe')
>> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field 
>> 'gamma_get' specified in initializer
 .gamma_get = armada_drm_crtc_gamma_get,
 ^
   drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from 
incompatible pointer type [-Werror=incompatible-pointer-types]
 .gamma_get = armada_drm_crtc_gamma_get,
  ^
   drivers/gpu//drm/armada/armada_fbdev.c:122:15: note: (near initialization 
for 'armada_fb_helper_funcs.initial_config')
   cc1: some warnings being treated as errors

vim +/gamma_set +121 drivers/gpu//drm/armada/armada_fbdev.c

96f60e37 Russell King   2012-08-15  115 ret = 1;
96f60e37 Russell King   2012-08-15  116 }
96f60e37 Russell King   2012-08-15  117 return ret;
96f60e37 Russell King   2012-08-15  118  }
96f60e37 Russell King   2012-08-15  119  
3a493879 Thierry Reding 2014-06-27  120  static const struct 
drm_fb_helper_funcs armada_fb_helper_funcs = {
96f60e37 Russell King   2012-08-15 @121 .gamma_set  = 
armada_drm_crtc_gamma_set,
96f60e37 Russell King   2012-08-15 @122 .gamma_get  = 
armada_drm_crtc_gamma_get,
96f60e37 Russell King   2012-08-15  123 .fb_probe   = 
armada_fb_probe,
96f60e37 Russell King   2012-08-15  124  };
96f60e37 Russell King   2012-08-15  125  

:: The code at line 121 was first introduced by commit
:: 96f60e37dc66091bde8d5de136ff6fda09f2d799 DRM: Armada: Add Armada DRM 
driver

:: TO: Russell King 
:: CC: Russell King 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/4] drm/vc4: Allow vblank_disable_immediate on non-fw-kms.

[Eric Anholt]

Mario Kleiner  writes:

> With instantaneous high precision vblank timestamping
> that updates at leading edge of vblank, the emulated
> "hw vblank counter" from vblank timestamping which
> increments at leading edge of vblank, and reliable
> page flip execution and completion at leading edge
> of vblank, we should meet the requirements for fast
> vblank irq disable/enable.
>
> Testing against rpi-4.12-rc5 Linux kernel with timing
> measurement equipment indicates this works fine,
> so allow immediate vblank disable for power saving.
>
> For debugging in case of unexpected trouble, booting
> with kernel cmdline option drm.vblankoffdelay=0
> would keep vblank irqs on to approximate old behavior.
>
> Signed-off-by: Mario Kleiner 
> Cc: Eric Anholt 

If you can spin this against drm-misc-next instead of the downstream
tree, I can get it applied.


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

RE: [PATCH] drm/amdgpu: export gpu always on cu bitmap

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Flora Cui
> Sent: Wednesday, June 21, 2017 3:42 AM
> To: Alex Deucher
> Cc: amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: export gpu always on cu bitmap
> 
> On Tue, Jun 20, 2017 at 10:47:39AM -0400, Alex Deucher wrote:
> > On Tue, Jun 20, 2017 at 4:15 AM, Flora Cui  wrote:
> > > Change-Id: I056d8af23340d46e5140bd10cc38dfb887cc78ab
> > > Signed-off-by: Flora Cui 
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +--
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
> > >  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   | 4 +++-
> > >  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 4 +++-
> > >  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 4 +++-
> > >  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 4 +++-
> > >  include/uapi/drm/amdgpu_drm.h   | 2 +-
> > >  8 files changed, 22 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > index 6b7d2a1..6b9a91c 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > @@ -1029,12 +1029,15 @@ struct amdgpu_gfx_config {
> > >  };
> > >
> > >  struct amdgpu_cu_info {
> > > -   uint32_t number; /* total active CU number */
> > > -   uint32_t ao_cu_mask;
> > > uint32_t max_waves_per_simd;
> > > uint32_t wave_front_size;
> > > uint32_t max_scratch_slots_per_cu;
> > > uint32_t lds_size;
> > > +
> > > +   /* total active CU number */
> > > +   uint32_t number;
> > > +   uint32_t ao_cu_mask;
> > > +   uint32_t ao_cu_bitmap[4][4];
> > > uint32_t bitmap[4][4];
> > >  };
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > index 0424711..5a1d794 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > @@ -67,9 +67,10 @@
> > >   * - 3.15.0 - Export more gpu info for gfx9
> > >   * - 3.16.0 - Add reserved vmid support
> > >   * - 3.17.0 - Add AMDGPU_NUM_VRAM_CPU_PAGE_FAULTS.
> > > + * - 3.18.0 - Export gpu always on cu bitmap
> > >   */
> > >  #define KMS_DRIVER_MAJOR   3
> > > -#define KMS_DRIVER_MINOR   17
> > > +#define KMS_DRIVER_MINOR   18
> > >  #define KMS_DRIVER_PATCHLEVEL  0
> > >
> > >  int amdgpu_vram_limit = 0;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > index f68ced6..6ba8204 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > @@ -591,8 +591,9 @@ static int amdgpu_info_ioctl(struct drm_device
> *dev, void *data, struct drm_file
> > > dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE;
> > >
> > > dev_info.cu_active_number = adev->gfx.cu_info.number;
> > > -   dev_info.cu_ao_mask = adev->gfx.cu_info.ao_cu_mask;
> > > dev_info.ce_ram_size = adev->gfx.ce_ram_size;
> > > +   memcpy(&dev_info.cu_ao_bitmap[0], &adev-
> >gfx.cu_info.ao_cu_bitmap[0],
> > > +  sizeof(adev->gfx.cu_info.ao_cu_bitmap));
> > > memcpy(&dev_info.cu_bitmap[0], &adev-
> >gfx.cu_info.bitmap[0],
> > >sizeof(adev->gfx.cu_info.bitmap));
> > > dev_info.vram_type = adev->mc.vram_type;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > > index 7b0b3cf..5173ca1 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > > @@ -3535,7 +3535,9 @@ static void gfx_v6_0_get_cu_info(struct
> amdgpu_device *adev)
> > > mask <<= 1;
> > > }
> > > active_cu_number += counter;
> > > -   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
> > > +   if (i < 2 && j < 2)
> > > +   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 
> > > 8));
> > > +   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
> > > }
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > index fb0a94c..8c4dd7b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > > @@ -5427,7 +5427,9 @@ static void gfx_v7_0_get_cu_info(struct
> amdgpu_device *adev)
> > > mask <<= 1;
> > > }
> > > active_cu_number += counter;
> > > -   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
> > > +   if (i < 2 && j < 2)
> > > +   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 
> > > 8))

Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

On 2017-06-21 09:38, Daniel Vetter wrote:
> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> totally obsolete.
>>
>> I think the gamma_store can end up invalid on error. But the way I read
>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>> this pesky legacy fbdev stuff be any better?
>>
>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
> 
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.

Then 3 patches would be needed, first some hybrid thing that does it the
old way, but also stores the lut in .gamma_store, then the split-out that
removes drm_fb_helper_save_lut_atomic, then whatever is needed to get
to the desired code. I can certainly do that, but do you want me to?

I.e., the statement that drm_fb_helper_save_lut_atomic is a nop is only
true when (part of) the other patch is also considered.

>> Signed-off-by: Peter Rosin 
> 
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 131 
>> 
>>  1 file changed, 40 insertions(+), 91 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 574af01..cc2d55d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct 
>> drm_fb_helper *fb_helper,
>>  }
>>  EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>>  
>> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct 
>> drm_fb_helper *helper)
>> -{
>> -uint16_t *r_base, *g_base, *b_base;
>> -int i;
>> -
>> -if (helper->funcs->gamma_get == NULL)
>> -return;
>> -
>> -r_base = crtc->gamma_store;
>> -g_base = r_base + crtc->gamma_size;
>> -b_base = g_base + crtc->gamma_size;
>> -
>> -for (i = 0; i < crtc->gamma_size; i++)
>> -helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], 
>> &b_base[i], i);
>> -}
>> -
>>  static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>>  {
>>  uint16_t *r_base, *g_base, *b_base;
>> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>>  if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
>>  continue;
>>  
>> -drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
>>  funcs->mode_set_base_atomic(mode_set->crtc,
>>  mode_set->fb,
>>  mode_set->x,
>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct 
>> drm_fb_helper *fb_helper,
>>  }
>>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>  
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> -struct drm_fb_helper *fb_helper = info->par;
>> -struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> -if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> 
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.

Oops, sorry, I simply missed that, I'll have a closer look...

> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.
> 
>> -u32 *palette;
>> -u32 value;
>> -/* place color in psuedopalette */
>> -if (regno > 16)
>> -return -EINVAL;
>> -palette = (u32 *)info->pseudo_palette;
>> -red >>= (16 - info->var.red.length);
>> -green >>= (16 - info->var.green.length);
>> -blue >>= (16 - info->var.blue.length);
>> -value = (red << info->var.red.offset) |
>> -(green << info->var.green.offset) |
>> -(blue << info->var.blue.offset);
>> -if (info->var.transp.length > 0) {
>> -u32 mask = (1 << info->var.transp.length) - 1;
>> -
>> -mask <<= info->var.transp.offset;
>> -value |= mask;
>> -}
>> -palette[regno] = value;
>> -return 0;
>> -}
>> -
>> -/*
>> - * The driver really shouldn't advertise pseudo/directcolor
>> - * visuals if it can't deal with the palette.
>> - */
>> -if (WARN_ON(!fb_helper->funcs->gamma_set ||
>> -!fb_helper->funcs->gamma_get))
>> -return -EINVAL;
>> -
>> -WARN_ON(fb->format->cpp[0] != 1);
>> -
>> -fb_helper-

Re: [PATCH 08/11] drm: nouveau: remove dead code and pointless local lut storage

[Peter Rosin]

On 2017-06-20 21:25, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code and hook up the crtc .gamma_set
> to use the crtc gamma_store directly instead of duplicating that
> info locally.

[...]

> - for (i = 0; i < 256; i++) {
> - u16 r = nv_crtc->lut.r[i] >> 2;
> - u16 g = nv_crtc->lut.g[i] >> 2;
> - u16 b = nv_crtc->lut.b[i] >> 2;
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
>  
> + for (i = 0; i < 256; i++) {
>   if (disp->disp->oclass < GF110_DISP) {
> - writew(r + 0x, lut + (i * 0x08) + 0);
> - writew(g + 0x, lut + (i * 0x08) + 2);
> - writew(b + 0x, lut + (i * 0x08) + 4);
> + writew((*r++ >> 2) + 0x, lut + (i * 0x08) + 0);
> + writew((*g++ >> 2) + 0x, lut + (i * 0x08) + 2);
> + writew((*b++ >> 2) + 0x, lut + (i * 0x08) + 4);
>   } else {
> - writew(r + 0x6000, lut + (i * 0x20) + 0);
> - writew(g + 0x6000, lut + (i * 0x20) + 2);
> - writew(b + 0x6000, lut + (i * 0x20) + 4);
> + writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
> + writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
> + writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
>   }
>   }
>  }

I forgot to mention this, but the above is very strange for
disp->disp->oclass >= GF110_DISP because 0x6000 interferes with
the 14 bits that appear to be the lut depth in the registers.

I suspect some other bit-shift should be used for that case?

Someone should probably consult a datasheet...

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

Re: [PATCH] drm/amdgpu: export gpu always on cu bitmap

[Christian König]

Am 21.06.2017 um 09:42 schrieb Flora Cui:

On Tue, Jun 20, 2017 at 10:47:39AM -0400, Alex Deucher wrote:

On Tue, Jun 20, 2017 at 4:15 AM, Flora Cui  wrote:

Change-Id: I056d8af23340d46e5140bd10cc38dfb887cc78ab
Signed-off-by: Flora Cui 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   | 4 +++-
  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 4 +++-
  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 4 +++-
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 4 +++-
  include/uapi/drm/amdgpu_drm.h   | 2 +-
  8 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6b7d2a1..6b9a91c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1029,12 +1029,15 @@ struct amdgpu_gfx_config {
  };

  struct amdgpu_cu_info {
-   uint32_t number; /* total active CU number */
-   uint32_t ao_cu_mask;
 uint32_t max_waves_per_simd;
 uint32_t wave_front_size;
 uint32_t max_scratch_slots_per_cu;
 uint32_t lds_size;
+
+   /* total active CU number */
+   uint32_t number;
+   uint32_t ao_cu_mask;
+   uint32_t ao_cu_bitmap[4][4];
 uint32_t bitmap[4][4];
  };

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0424711..5a1d794 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -67,9 +67,10 @@
   * - 3.15.0 - Export more gpu info for gfx9
   * - 3.16.0 - Add reserved vmid support
   * - 3.17.0 - Add AMDGPU_NUM_VRAM_CPU_PAGE_FAULTS.
+ * - 3.18.0 - Export gpu always on cu bitmap
   */
  #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   17
+#define KMS_DRIVER_MINOR   18
  #define KMS_DRIVER_PATCHLEVEL  0

  int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f68ced6..6ba8204 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -591,8 +591,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
 dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE;

 dev_info.cu_active_number = adev->gfx.cu_info.number;
-   dev_info.cu_ao_mask = adev->gfx.cu_info.ao_cu_mask;
 dev_info.ce_ram_size = adev->gfx.ce_ram_size;
+   memcpy(&dev_info.cu_ao_bitmap[0], 
&adev->gfx.cu_info.ao_cu_bitmap[0],
+  sizeof(adev->gfx.cu_info.ao_cu_bitmap));
 memcpy(&dev_info.cu_bitmap[0], &adev->gfx.cu_info.bitmap[0],
sizeof(adev->gfx.cu_info.bitmap));
 dev_info.vram_type = adev->mc.vram_type;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 7b0b3cf..5173ca1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -3535,7 +3535,9 @@ static void gfx_v6_0_get_cu_info(struct amdgpu_device 
*adev)
 mask <<= 1;
 }
 active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
 }
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index fb0a94c..8c4dd7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -5427,7 +5427,9 @@ static void gfx_v7_0_get_cu_info(struct amdgpu_device 
*adev)
 mask <<= 1;
 }
 active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
 }
 }
 gfx_v7_0_select_se_sh(adev, 0x, 0x, 0x);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 1a75ab1..9edb509 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -7080,7 +7080,9 @@ static void gfx_v8_0_get_cu_info(struct amdgpu_device 
*adev)
 mask <<= 1;
 }
 active_cu_number += counter;
-   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
+   if (i < 2 && j < 2)
+   ao_cu_mask |= (ao_bitm

Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Michel Dänzer]

On 21/06/17 05:14 PM, Daniel Vetter wrote:
> On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
>> On 21/06/17 04:38 PM, Daniel Vetter wrote:
>>> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
 This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
 totally obsolete.

 I think the gamma_store can end up invalid on error. But the way I read
 it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
 this pesky legacy fbdev stuff be any better?

 drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
 it saves it to the gamma_store which should already be up to date with what
 .gamma_get would return and is thus a nop. So, zap it.
>>>
>>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
>>> think.
>>>  
 Signed-off-by: Peter Rosin 
>>
>> [...]
>>
 @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct 
 drm_fb_helper *fb_helper,
  }
  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
  
 -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
 -   u16 blue, u16 regno, struct fb_info *info)
 -{
 -  struct drm_fb_helper *fb_helper = info->par;
 -  struct drm_framebuffer *fb = fb_helper->fb;
 -
 -  if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>>>
>>> This case here seems gone, and it was also the pièce de résistance when I
>>> tried tackling fbdev lut support. As far as I understand this stuff we do
>>> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
>>> pointless. But I'm honestly not really clear.
>>>
>>> I think removing this case should also be a separate patch, and I'd very
>>> much prefer that someone with some fbdev-clue would ack it.
>>
>> No need for anyone with fbdev-clue, just run fbset -i. :)

Adding Bartlomiej and the linux-fbdev list, just in case I'm wrong below.


>> fbcon uses a TRUECOLOR visual at depths > 8.
> 
> Then I understand even less what exactly this code does ... Should we keep
> it?

I think we need it. It updates the entries of info->pseudo_palette,
which is kind of a pseudocolor palette mapping 16 indices to pixel
values to write to the framebuffer.

If the setcolreg hook is removed, the setcmap hook needs to do this instead.


-- 
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: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
> On 21/06/17 04:38 PM, Daniel Vetter wrote:
> > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >> totally obsolete.
> >>
> >> I think the gamma_store can end up invalid on error. But the way I read
> >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> >> this pesky legacy fbdev stuff be any better?
> >>
> >> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> >> it saves it to the gamma_store which should already be up to date with what
> >> .gamma_get would return and is thus a nop. So, zap it.
> > 
> > Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> > think.
> >  
> >> Signed-off-by: Peter Rosin 
> 
> [...]
> 
> >> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct 
> >> drm_fb_helper *fb_helper,
> >>  }
> >>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
> >>  
> >> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> >> -   u16 blue, u16 regno, struct fb_info *info)
> >> -{
> >> -  struct drm_fb_helper *fb_helper = info->par;
> >> -  struct drm_framebuffer *fb = fb_helper->fb;
> >> -
> >> -  if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> > 
> > This case here seems gone, and it was also the pièce de résistance when I
> > tried tackling fbdev lut support. As far as I understand this stuff we do
> > not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> > pointless. But I'm honestly not really clear.
> > 
> > I think removing this case should also be a separate patch, and I'd very
> > much prefer that someone with some fbdev-clue would ack it.
> 
> No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
> TRUECOLOR visual at depths > 8.

Then I understand even less what exactly this code does ... Should we keep
it? Is it just pure cargo-cult? Only needed when we'd change the color
depth of the fbdev buffer, which we never do at runtime?

> > It's a pre-existing bug, but should we also try to restore the fbdev lut
> > in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> > but might be relevant for your use-case. Just try to run both an fbdev
> > application and some kms-native thing, and then SIGKILL the native kms
> > app.
> > 
> > But since pre-existing not really required, and probably too much effort.
> 
> I suspect something like that indeed needs to be done though. E.g.
> killing Xorg results in fbcon continuing to use the LUT set by Xorg.

Ok, the only trouble with that is atomic kms locking, which requires that
we can only do 1 commit per lock-holding time, and also
drm_modeset_lock_all is an evil hack and needs to be phased out. Two
solutions:

- We just update the lut after we've dropped the locks again in
  restore_fbdev_mode.
- We need both a legacy path, in restore_fbdev_mode_legacy, and an atomic
  path in restore_fbdev_mode_atomic. The latter cannot use the gamme_set
  callback, since that would call drm_atomic_helper_legacy_gamma_set for
  atomic drivers, which would result in two calls to drm_atomic_commit in
  one critical sections. Instead that needs to open-code the logic in
  drm_atomic_helper_legacy_gamma_set and integrate it into the overall
  fbdev restore commit.

The 2nd option is a bit more typing, but much cleaner. It also fits better
into some of the refactoring plans I have (I want to get rid of
drm_modeset_lock_all in the fbdev emulation). And has the benefit of
giving drivers a bit more complex fbdev emulation atomic commits, which
would be good for testing I think.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Michel Dänzer]

On 21/06/17 04:38 PM, Daniel Vetter wrote:
> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> totally obsolete.
>>
>> I think the gamma_store can end up invalid on error. But the way I read
>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>> this pesky legacy fbdev stuff be any better?
>>
>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
> 
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.
>  
>> Signed-off-by: Peter Rosin 

[...]

>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct 
>> drm_fb_helper *fb_helper,
>>  }
>>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>  
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> -struct drm_fb_helper *fb_helper = info->par;
>> -struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> -if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> 
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.
> 
> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.

No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
TRUECOLOR visual at depths > 8.


> It's a pre-existing bug, but should we also try to restore the fbdev lut
> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> but might be relevant for your use-case. Just try to run both an fbdev
> application and some kms-native thing, and then SIGKILL the native kms
> app.
> 
> But since pre-existing not really required, and probably too much effort.

I suspect something like that indeed needs to be done though. E.g.
killing Xorg results in fbcon continuing to use the LUT set by Xorg.


-- 
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] drm/amdgpu: export gpu always on cu bitmap

[Flora Cui]

On Tue, Jun 20, 2017 at 10:47:39AM -0400, Alex Deucher wrote:
> On Tue, Jun 20, 2017 at 4:15 AM, Flora Cui  wrote:
> > Change-Id: I056d8af23340d46e5140bd10cc38dfb887cc78ab
> > Signed-off-by: Flora Cui 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +--
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
> >  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   | 4 +++-
> >  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 4 +++-
> >  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 4 +++-
> >  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 4 +++-
> >  include/uapi/drm/amdgpu_drm.h   | 2 +-
> >  8 files changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 6b7d2a1..6b9a91c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -1029,12 +1029,15 @@ struct amdgpu_gfx_config {
> >  };
> >
> >  struct amdgpu_cu_info {
> > -   uint32_t number; /* total active CU number */
> > -   uint32_t ao_cu_mask;
> > uint32_t max_waves_per_simd;
> > uint32_t wave_front_size;
> > uint32_t max_scratch_slots_per_cu;
> > uint32_t lds_size;
> > +
> > +   /* total active CU number */
> > +   uint32_t number;
> > +   uint32_t ao_cu_mask;
> > +   uint32_t ao_cu_bitmap[4][4];
> > uint32_t bitmap[4][4];
> >  };
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 0424711..5a1d794 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -67,9 +67,10 @@
> >   * - 3.15.0 - Export more gpu info for gfx9
> >   * - 3.16.0 - Add reserved vmid support
> >   * - 3.17.0 - Add AMDGPU_NUM_VRAM_CPU_PAGE_FAULTS.
> > + * - 3.18.0 - Export gpu always on cu bitmap
> >   */
> >  #define KMS_DRIVER_MAJOR   3
> > -#define KMS_DRIVER_MINOR   17
> > +#define KMS_DRIVER_MINOR   18
> >  #define KMS_DRIVER_PATCHLEVEL  0
> >
> >  int amdgpu_vram_limit = 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index f68ced6..6ba8204 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -591,8 +591,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, 
> > void *data, struct drm_file
> > dev_info.gart_page_size = AMDGPU_GPU_PAGE_SIZE;
> >
> > dev_info.cu_active_number = adev->gfx.cu_info.number;
> > -   dev_info.cu_ao_mask = adev->gfx.cu_info.ao_cu_mask;
> > dev_info.ce_ram_size = adev->gfx.ce_ram_size;
> > +   memcpy(&dev_info.cu_ao_bitmap[0], 
> > &adev->gfx.cu_info.ao_cu_bitmap[0],
> > +  sizeof(adev->gfx.cu_info.ao_cu_bitmap));
> > memcpy(&dev_info.cu_bitmap[0], &adev->gfx.cu_info.bitmap[0],
> >sizeof(adev->gfx.cu_info.bitmap));
> > dev_info.vram_type = adev->mc.vram_type;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > index 7b0b3cf..5173ca1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > @@ -3535,7 +3535,9 @@ static void gfx_v6_0_get_cu_info(struct amdgpu_device 
> > *adev)
> > mask <<= 1;
> > }
> > active_cu_number += counter;
> > -   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
> > +   if (i < 2 && j < 2)
> > +   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 
> > 8));
> > +   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
> > }
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index fb0a94c..8c4dd7b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -5427,7 +5427,9 @@ static void gfx_v7_0_get_cu_info(struct amdgpu_device 
> > *adev)
> > mask <<= 1;
> > }
> > active_cu_number += counter;
> > -   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
> > +   if (i < 2 && j < 2)
> > +   ao_cu_mask |= (ao_bitmap << (i * 16 + j * 
> > 8));
> > +   cu_info->ao_cu_bitmap[i][j] = ao_bitmap;
> > }
> > }
> > gfx_v7_0_select_se_sh(adev, 0x, 0x, 0x);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 1a75ab1..9edb509 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -7080,7 +7080

Re: [PATCH 00/11] improve the fb_setcmap helper

[Daniel Vetter]

On Tue, Jun 20, 2017 at 09:25:24PM +0200, Peter Rosin wrote:
> Hi!
> 
> While trying to get CLUT support for the atmel_hlcdc driver, and
> specifically for the emulated fbdev interface, I received some
> push-back that my feeble in-driver attempts should be solved
> by the core. This is my attempt to do it right.
> 
> Boris and Daniel, was this approximately what you had in mind?

Yeah, this is awesome. I tried to do it a few times myself, but always
failed (also due to lack of real use-case on my side).

> I have obviously not tested all of this with more than a compile,
> but the first patch is enough to make the atmel-hlcdc driver
> do what I need. The rest is just lots of removals and cleanup
> made possible by the improved core.

If it works for you it's imo good enough. Not sure anyone else really
cares about fbdev lut support at all. I have a few comments on the first
patch, but once that's sorted, and once we have given driver maintainers
enough time to ack I think I'll merge the entire pile into drm-misc.

Nice work, thanks for doing it.

Cheers, Daniel

> Please test, I would not be surprised if I have fouled up some
> bit-manipulation somewhere in this mostly mechanical change...
> 
> Cheers,
> peda
> 
> Peter Rosin (11):
>   drm/fb-helper: do a generic fb_setcmap helper in terms of crtc
> .gamma_set
>   drm: amd: remove dead code and pointless local lut storage
>   drm: ast: remove dead code and pointless local lut storage
>   drm: cirrus: remove dead code and pointless local lut storage
>   dmr: gma500: remove dead code and pointless local lut storage
>   drm: i915: remove dead code and pointless local lut storage
>   drm: mgag200: remove dead code and pointless local lut storage
>   drm: nouveau: remove dead code and pointless local lut storage
>   drm: radeon: remove dead code and pointless local lut storage
>   drm: stm: remove dead code and pointless local lut storage
>   drm: remove unused and redundant callbacks
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  24 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h|   1 -
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  |  27 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  |  27 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c   |  27 ++
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   |  27 ++
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c|  23 -
>  drivers/gpu/drm/ast/ast_drv.h   |   1 -
>  drivers/gpu/drm/ast/ast_fb.c|  20 -
>  drivers/gpu/drm/ast/ast_mode.c  |  26 ++
>  drivers/gpu/drm/cirrus/cirrus_drv.h |   8 --
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c   |   2 -
>  drivers/gpu/drm/cirrus/cirrus_mode.c|  71 ---
>  drivers/gpu/drm/drm_fb_helper.c | 131 
> +---
>  drivers/gpu/drm/gma500/framebuffer.c|  22 -
>  drivers/gpu/drm/gma500/gma_display.c|  32 +++
>  drivers/gpu/drm/gma500/psb_intel_display.c  |   7 +-
>  drivers/gpu/drm/gma500/psb_intel_drv.h  |   1 -
>  drivers/gpu/drm/i915/intel_drv.h|   1 -
>  drivers/gpu/drm/i915/intel_fbdev.c  |  31 ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h   |   5 --
>  drivers/gpu/drm/mgag200/mgag200_fb.c|   2 -
>  drivers/gpu/drm/mgag200/mgag200_mode.c  |  62 -
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c |  26 ++
>  drivers/gpu/drm/nouveau/nouveau_crtc.h  |   3 -
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |  22 -
>  drivers/gpu/drm/nouveau/nv50_display.c  |  39 +++--
>  drivers/gpu/drm/radeon/atombios_crtc.c  |   1 -
>  drivers/gpu/drm/radeon/radeon_connectors.c  |   7 +-
>  drivers/gpu/drm/radeon/radeon_display.c |  71 ++-
>  drivers/gpu/drm/radeon/radeon_fb.c  |   2 -
>  drivers/gpu/drm/radeon/radeon_legacy_crtc.c |   1 -
>  drivers/gpu/drm/stm/ltdc.c  |  12 ---
>  drivers/gpu/drm/stm/ltdc.h  |   1 -
>  include/drm/drm_fb_helper.h |  32 ---
>  include/drm/drm_modeset_helper_vtables.h|  16 
>  36 files changed, 171 insertions(+), 640 deletions(-)
> 
> -- 
> 2.1.4
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Daniel Vetter]

On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> totally obsolete.
> 
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
> 
> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> it saves it to the gamma_store which should already be up to date with what
> .gamma_get would return and is thus a nop. So, zap it.

Removing drm_fb_helper_save_lut_atomic should be a separate patch I
think.
 
> Signed-off-by: Peter Rosin 

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 131 
> 
>  1 file changed, 40 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..cc2d55d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct 
> drm_fb_helper *fb_helper,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>  
> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct 
> drm_fb_helper *helper)
> -{
> - uint16_t *r_base, *g_base, *b_base;
> - int i;
> -
> - if (helper->funcs->gamma_get == NULL)
> - return;
> -
> - r_base = crtc->gamma_store;
> - g_base = r_base + crtc->gamma_size;
> - b_base = g_base + crtc->gamma_size;
> -
> - for (i = 0; i < crtc->gamma_size; i++)
> - helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], 
> &b_base[i], i);
> -}
> -
>  static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>  {
>   uint16_t *r_base, *g_base, *b_base;
> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>   if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
>   continue;
>  
> - drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
>   funcs->mode_set_base_atomic(mode_set->crtc,
>   mode_set->fb,
>   mode_set->x,
> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct 
> drm_fb_helper *fb_helper,
>  }
>  EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>  
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> -  u16 blue, u16 regno, struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - struct drm_framebuffer *fb = fb_helper->fb;
> -
> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {

This case here seems gone, and it was also the pièce de résistance when I
tried tackling fbdev lut support. As far as I understand this stuff we do
not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
pointless. But I'm honestly not really clear.

I think removing this case should also be a separate patch, and I'd very
much prefer that someone with some fbdev-clue would ack it.

> - u32 *palette;
> - u32 value;
> - /* place color in psuedopalette */
> - if (regno > 16)
> - return -EINVAL;
> - palette = (u32 *)info->pseudo_palette;
> - red >>= (16 - info->var.red.length);
> - green >>= (16 - info->var.green.length);
> - blue >>= (16 - info->var.blue.length);
> - value = (red << info->var.red.offset) |
> - (green << info->var.green.offset) |
> - (blue << info->var.blue.offset);
> - if (info->var.transp.length > 0) {
> - u32 mask = (1 << info->var.transp.length) - 1;
> -
> - mask <<= info->var.transp.offset;
> - value |= mask;
> - }
> - palette[regno] = value;
> - return 0;
> - }
> -
> - /*
> -  * The driver really shouldn't advertise pseudo/directcolor
> -  * visuals if it can't deal with the palette.
> -  */
> - if (WARN_ON(!fb_helper->funcs->gamma_set ||
> - !fb_helper->funcs->gamma_get))
> - return -EINVAL;
> -
> - WARN_ON(fb->format->cpp[0] != 1);
> -
> - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> - return 0;
> -}
> -
>  /**
>   * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>   * @cmap: cmap to set
> @@ -1220,51 +1159,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, 
> struct fb_info *info)
>  {
>   struct drm_fb_helper *fb_helper = info->par;
>   struct drm_device *dev = fb_helper->dev;
> - const struct drm_crtc_helper_funcs *crtc_funcs;
> - u16 *red, *green, *blue, *transp;
> + struct drm_modeset_acquire_ctx ctx;
>   struct drm_crtc *crtc;
> - int i, j, rc = 0;
> - int

Re: [PATCH 2/4] drm/radeon: Allow vblank_disable_immediate.

[Michel Dänzer]

On 21/06/17 10:44 AM, Mario Kleiner wrote:
> With instantaneous high precision vblank timestamping
> that updates at leading edge of vblank, a cooked hw
> vblank counter which increments at leading edge of
> vblank, and reliable page flip execution and completion
> at leading edge of vblank, we should meet the requirements
> for fast/immediate vblank irq disable/enable.
> 
> Testing on Linux-4.12-rc5 + drm-next on a Radeon HD 5770
> (DCE 4) with timing measurement equipment indicates this
> works fine, so allow immediate vblank disable for power
> saving.
> 
> For debugging in case of unexpected trouble, booting
> with kernel cmdline option drm.vblankoffdelay=0
> (or echo 0 > /sys/module/drm/parameters/vblankoffdelay)
> would keep vblank irqs permanently on to approximate old
> behavior.
> 
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 
> Cc: Michel Dänzer 

My only doubt is whether this is also reliable on older (e.g. pre-R600)
GPUs. For newer GPUs (tested on Kaveri):

Reviewed-and-Tested-by: Michel Dänzer 


-- 
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 3/4] drm/amdgpu: Allow vblank_disable_immediate.

[Michel Dänzer]

On 21/06/17 10:44 AM, Mario Kleiner wrote:
> With instantaneous high precision vblank timestamping
> that updates at leading edge of vblank, a cooked hw
> vblank counter which increments at leading edge of
> vblank, and reliable page flip execution and completion
> at leading edge of vblank, we should meet the requirements
> for fast/immediate vblank irq disable/enable.
> 
> Testing on Linux-4.12-rc5 + drm-next on a Radeon R9 380
> Tonga Pro (DCE 10) with timing measurement equipment
> indicates this works fine, so allow immediate vblank
> disable for power saving.
> 
> For debugging in case of unexpected trouble, booting
> with kernel cmdline option drm.vblankoffdelay=0
> (or echo 0 > /sys/module/drm/parameters/vblankoffdelay)
> would keep vblank irqs permanently on to approximate old
> behavior.
> 
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 
> Cc: Michel Dänzer 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 62da6c5..a28f8aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -220,6 +220,10 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>   int r = 0;
>  
>   spin_lock_init(&adev->irq.lock);
> +
> + /* Disable vblank irqs aggressively for power-saving */
> + adev->ddev->vblank_disable_immediate = true;
> +
>   r = drm_vblank_init(adev->ddev, adev->mode_info.num_crtc);
>   if (r) {
>   return r;
> 

Reviewed-by: Michel Dänzer 


-- 
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 libdrm] amdgpu: update always on cu bitmap

[Daniel Vetter]

On Wed, Jun 21, 2017 at 10:13:54AM +0800, Flora Cui wrote:
> each SE take 16 bit in cu_ao_mask. For ASICs with 4 SE, cu_ao_mask
> has invalid value. so I change cu_ao_mask to cu_ao_bitmap[4][4] and increase
> kmd driver version.

Please read Documentation/ioctl/botching-up-ioctls.txt

You cannot change ioctl structures like you're proposing here.

Thanks, Daniel
> 
> On Tue, Jun 20, 2017 at 11:49:23AM +0200, Christian König wrote:
> > I'm not 100% sure what this is all about, but it clearly won't work like
> > this.
> > 
> > >diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
> > >index df250de..dcbe22c 100644
> > >--- a/include/drm/amdgpu_drm.h
> > >+++ b/include/drm/amdgpu_drm.h
> > >@@ -832,7 +832,7 @@ struct drm_amdgpu_info_device {
> > >   __u64 max_memory_clock;
> > >   /* cu information */
> > >   __u32 cu_active_number;
> > >-  __u32 cu_ao_mask;
> > >+  __u32 cu_ao_bitmap[4][4];
> > >   __u32 cu_bitmap[4][4];
> > >   /** Render backend pipe mask. One render backend is CB+DB. */
> > >   __u32 enabled_rb_pipes_mask;
> > That is a non-backward compatible change to the kernel interface and as such
> > forbidden.
> > 
> > Regards,
> > Christian.
> > 
> > Am 20.06.2017 um 11:04 schrieb Flora Cui:
> > >Change-Id: Ie2a812716a6802f7a5a0bc09b1a8db824c5bf2ed
> > >Signed-off-by: Flora Cui 
> > >---
> > >  amdgpu/amdgpu.h  | 2 +-
> > >  amdgpu/amdgpu_gpu_info.c | 2 +-
> > >  include/drm/amdgpu_drm.h | 2 +-
> > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > >diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> > >index b6779f9..34ca5f1 100644
> > >--- a/amdgpu/amdgpu.h
> > >+++ b/amdgpu/amdgpu.h
> > >@@ -486,7 +486,7 @@ struct amdgpu_gpu_info {
> > >   uint32_t pa_sc_raster_cfg1[4];
> > >   /* CU info */
> > >   uint32_t cu_active_number;
> > >-  uint32_t cu_ao_mask;
> > >+  uint32_t cu_ao_bitmap[4][4];
> > >   uint32_t cu_bitmap[4][4];
> > >   /* video memory type info*/
> > >   uint32_t vram_type;
> > >diff --git a/amdgpu/amdgpu_gpu_info.c b/amdgpu/amdgpu_gpu_info.c
> > >index 34f77be..acfd700 100644
> > >--- a/amdgpu/amdgpu_gpu_info.c
> > >+++ b/amdgpu/amdgpu_gpu_info.c
> > >@@ -229,7 +229,7 @@ drm_private int 
> > >amdgpu_query_gpu_info_init(amdgpu_device_handle dev)
> > >   }
> > >   dev->info.cu_active_number = dev->dev_info.cu_active_number;
> > >-  dev->info.cu_ao_mask = dev->dev_info.cu_ao_mask;
> > >+  memcpy(&dev->info.cu_ao_bitmap[0][0], 
> > >&dev->dev_info.cu_ao_bitmap[0][0], sizeof(dev->info.cu_ao_bitmap));
> > >   memcpy(&dev->info.cu_bitmap[0][0], &dev->dev_info.cu_bitmap[0][0], 
> > > sizeof(dev->info.cu_bitmap));
> > >   /* TODO: info->max_quad_shader_pipes is not set */
> > >diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
> > >index df250de..dcbe22c 100644
> > >--- a/include/drm/amdgpu_drm.h
> > >+++ b/include/drm/amdgpu_drm.h
> > >@@ -832,7 +832,7 @@ struct drm_amdgpu_info_device {
> > >   __u64 max_memory_clock;
> > >   /* cu information */
> > >   __u32 cu_active_number;
> > >-  __u32 cu_ao_mask;
> > >+  __u32 cu_ao_bitmap[4][4];
> > >   __u32 cu_bitmap[4][4];
> > >   /** Render backend pipe mask. One render backend is CB+DB. */
> > >   __u32 enabled_rb_pipes_mask;
> > 
> > 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx