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

[axie]

Hi Marek,


So do you agree that spinlock disables CPU preemption, contrary to your 
original idea?



If you have new reason that this patch does not improve, please speak out.


Many patches in GPU driver aim at improving performance and power 
efficiency. Does most patches submitted in AMDGPU requires a 
benchmarking first?


If all developers are required to always answer your questions when code 
review, I am afraid that most open source community developers cannot 
meet that requirement and stop working on AMDGPU.



To improve performance, there are many bottlenecks to clear. When the 
last several bottlenecks are clear, the performance will show faster 
more significantly.


My pass profiling experience told me that clearing a lock can improve 
performance for some driver like 0.3% to much bigger percentage. It 
depends on many factors, even depends on the application itself.



This is not the first bottleneck fixed. This is surely not the last one.


Thanks,

Alex Bin


On 2017-06-22 07:54 PM, Marek Olšák wrote:

That's all nice, but does it improve performance? Have you been able
to measure some performance difference with that code? Were you
targeting a specific inefficiency you had seen e.g. with a CPU
profiler?

Marek

On Thu, Jun 22, 2017 at 8:19 PM, axie  wrote:

To clarify, local IRQ is disabled by calling raw_local_irq_save(flags);

Function __lock_acquire double checks that the local IRQ is really disabled.



On 2017-06-22 01:34 PM, axie wrote:

Hi Marek,

Spin lock and spin unlock is fast. But it is not so fast compared with
atomic, which is a single CPU instruction in x86.


1. spinlock does NOT allow preemption at local CPU. Let us have a look at
how spin lock was implemented.

static inline void __raw_spin_lock(raw_spinlock_t *lock)
{
 preempt_disable(); --This is
memory barrier operation too.
 spin_acquire(>dep_map, 0, 0, _RET_IP_);
 LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
}

2.  A function  __lock_acquire called by spinlock. The function is so long
that I would not attach all of it here.

There is atomic operation inside and 12 meta data updates and 14 if
statements and it calls quite some other functions.

Note that it disable IRQ...

static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
   int trylock, int read, int check, int hardirqs_off,
   struct lockdep_map *nest_lock, unsigned long ip,
   int references, int pin_count)
{
 struct task_struct *curr = current;
 struct lock_class *class = NULL;
 struct held_lock *hlock;
 unsigned int depth;
 int chain_head = 0;
 int class_idx;
 u64 chain_key;

 if (unlikely(!debug_locks))
 return 0;

 /*
  * Lockdep should run with IRQs disabled, otherwise we could
  * get an interrupt which would want to take locks, which would
  * end up in lockdep and have you got a head-ache already?
  */
 if (DEBUG_LOCKS_WARN_ON(!irqs_disabled())) <lockdep_recursion = 1;
 trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock,
ip);
 __lock_acquire(lock, subclass, trylock, read, check,
irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
 current->lockdep_recursion = 0;
 raw_local_irq_restore(flags);
}


Thanks,

Alex Bin


On 2017-06-22 12:27 PM, Marek Olšák wrote:

On Thu, Jun 22, 2017 at 5:33 PM, Xie, AlexBin 
wrote:

Hi Christian,


In fact, the change from spinlock to atomic is quite painful. When I
started, I thought it was easy but later I found there might be race
condition here and there. Now I think the change looks more robust. In
kernel source, there are several other drivers used the same trick.


On the other hand, I think the logic itself might be optimized
considering
the locking. But I had spent quite some effort to maintain original
logic.

It seems quite complicated and I don't know if there is any
performance benefit. Spinlocks are nice because they allow preemption.

It would be more interesting to merge the CS and BO_LIST ioctls into one.

Marek




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

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

[Marek Olšák]

That's all nice, but does it improve performance? Have you been able
to measure some performance difference with that code? Were you
targeting a specific inefficiency you had seen e.g. with a CPU
profiler?

Marek

On Thu, Jun 22, 2017 at 8:19 PM, axie  wrote:
> To clarify, local IRQ is disabled by calling raw_local_irq_save(flags);
>
> Function __lock_acquire double checks that the local IRQ is really disabled.
>
>
>
> On 2017-06-22 01:34 PM, axie wrote:
>>
>> Hi Marek,
>>
>> Spin lock and spin unlock is fast. But it is not so fast compared with
>> atomic, which is a single CPU instruction in x86.
>>
>>
>> 1. spinlock does NOT allow preemption at local CPU. Let us have a look at
>> how spin lock was implemented.
>>
>> static inline void __raw_spin_lock(raw_spinlock_t *lock)
>> {
>> preempt_disable(); --This is
>> memory barrier operation too.
>> spin_acquire(>dep_map, 0, 0, _RET_IP_);
>> LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
>> }
>>
>> 2.  A function  __lock_acquire called by spinlock. The function is so long
>> that I would not attach all of it here.
>>
>> There is atomic operation inside and 12 meta data updates and 14 if
>> statements and it calls quite some other functions.
>>
>> Note that it disable IRQ...
>>
>> static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
>>   int trylock, int read, int check, int hardirqs_off,
>>   struct lockdep_map *nest_lock, unsigned long ip,
>>   int references, int pin_count)
>> {
>> struct task_struct *curr = current;
>> struct lock_class *class = NULL;
>> struct held_lock *hlock;
>> unsigned int depth;
>> int chain_head = 0;
>> int class_idx;
>> u64 chain_key;
>>
>> if (unlikely(!debug_locks))
>> return 0;
>>
>> /*
>>  * Lockdep should run with IRQs disabled, otherwise we could
>>  * get an interrupt which would want to take locks, which would
>>  * end up in lockdep and have you got a head-ache already?
>>  */
>> if (DEBUG_LOCKS_WARN_ON(!irqs_disabled())) <<> return 0;
>>
>> 
>>
>> 3. Another function called by spinlock in a higher level:
>>
>> void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
>>
>>   int trylock, int read, int check,
>>   struct lockdep_map *nest_lock, unsigned long ip)
>> {
>> unsigned long flags;
>>
>> if (unlikely(current->lockdep_recursion))
>> return;
>>
>> raw_local_irq_save(flags);
>> check_flags(flags);
>>
>> current->lockdep_recursion = 1;
>> trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock,
>> ip);
>> __lock_acquire(lock, subclass, trylock, read, check,
>>irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
>> current->lockdep_recursion = 0;
>> raw_local_irq_restore(flags);
>> }
>>
>>
>> Thanks,
>>
>> Alex Bin
>>
>>
>> On 2017-06-22 12:27 PM, Marek Olšák wrote:
>>>
>>> On Thu, Jun 22, 2017 at 5:33 PM, Xie, AlexBin 
>>> wrote:

 Hi Christian,


 In fact, the change from spinlock to atomic is quite painful. When I
 started, I thought it was easy but later I found there might be race
 condition here and there. Now I think the change looks more robust. In
 kernel source, there are several other drivers used the same trick.


 On the other hand, I think the logic itself might be optimized
 considering
 the locking. But I had spent quite some effort to maintain original
 logic.
>>>
>>> It seems quite complicated and I don't know if there is any
>>> performance benefit. Spinlocks are nice because they allow preemption.
>>>
>>> It would be more interesting to merge the CS and BO_LIST ioctls into one.
>>>
>>> Marek
>>
>>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/psp: upper_32_bits/lower_32_bits for address setup

[Alex Deucher]

Rather than casting and shifting.  Fixes sparse case warnings.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 12 ++--
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 12 ++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index c224c5c..0b5f533 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -152,8 +152,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_gfx_cmd_resp 
*cmd,
 uint64_t tmr_mc, uint32_t size)
 {
cmd->cmd_id = GFX_CMD_ID_SETUP_TMR;
-   cmd->cmd.cmd_setup_tmr.buf_phy_addr_lo = (uint32_t)tmr_mc;
-   cmd->cmd.cmd_setup_tmr.buf_phy_addr_hi = (uint32_t)(tmr_mc >> 32);
+   cmd->cmd.cmd_setup_tmr.buf_phy_addr_lo = lower_32_bits(tmr_mc);
+   cmd->cmd.cmd_setup_tmr.buf_phy_addr_hi = upper_32_bits(tmr_mc);
cmd->cmd.cmd_setup_tmr.buf_size = size;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 20c1e53..2258323 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -96,8 +96,8 @@ int psp_v10_0_prep_cmd_buf(struct amdgpu_firmware_info 
*ucode, struct psp_gfx_cm
header = (struct common_firmware_header *)ucode->fw;
 
cmd->cmd_id = GFX_CMD_ID_LOAD_IP_FW;
-   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_lo = (uint32_t)fw_mem_mc_addr;
-   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_hi = 
(uint32_t)((uint64_t)fw_mem_mc_addr >> 32);
+   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_lo = lower_32_bits(fw_mem_mc_addr);
+   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_hi = upper_32_bits(fw_mem_mc_addr);
cmd->cmd.cmd_load_ip_fw.fw_size = le32_to_cpu(header->ucode_size_bytes);
 
ret = psp_v10_0_get_fw_type(ucode, >cmd.cmd_load_ip_fw.fw_type);
@@ -172,10 +172,10 @@ int psp_v10_0_cmd_submit(struct psp_context *psp,
write_frame = ring->ring_mem + (psp_write_ptr_reg / 
(sizeof(struct psp_gfx_rb_frame) / 4));
 
/* Update KM RB frame */
-   write_frame->cmd_buf_addr_hi = (unsigned int)(cmd_buf_mc_addr >> 32);
-   write_frame->cmd_buf_addr_lo = (unsigned int)(cmd_buf_mc_addr);
-   write_frame->fence_addr_hi = (unsigned int)(fence_mc_addr >> 32);
-   write_frame->fence_addr_lo = (unsigned int)(fence_mc_addr);
+   write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
+   write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
+   write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
+   write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
write_frame->fence_value = index;
 
/* Update the write Pointer in DWORDs */
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c 
b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 6e5c6ed..c98d77d 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -254,8 +254,8 @@ int psp_v3_1_prep_cmd_buf(struct amdgpu_firmware_info 
*ucode, struct psp_gfx_cmd
memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
 
cmd->cmd_id = GFX_CMD_ID_LOAD_IP_FW;
-   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_lo = (uint32_t)fw_mem_mc_addr;
-   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_hi = 
(uint32_t)((uint64_t)fw_mem_mc_addr >> 32);
+   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_lo = lower_32_bits(fw_mem_mc_addr);
+   cmd->cmd.cmd_load_ip_fw.fw_phy_addr_hi = upper_32_bits(fw_mem_mc_addr);
cmd->cmd.cmd_load_ip_fw.fw_size = ucode->ucode_size;
 
ret = psp_v3_1_get_fw_type(ucode, >cmd.cmd_load_ip_fw.fw_type);
@@ -375,10 +375,10 @@ int psp_v3_1_cmd_submit(struct psp_context *psp,
memset(write_frame, 0, sizeof(struct psp_gfx_rb_frame));
 
/* Update KM RB frame */
-   write_frame->cmd_buf_addr_hi = (unsigned int)(cmd_buf_mc_addr >> 32);
-   write_frame->cmd_buf_addr_lo = (unsigned int)(cmd_buf_mc_addr);
-   write_frame->fence_addr_hi = (unsigned int)(fence_mc_addr >> 32);
-   write_frame->fence_addr_lo = (unsigned int)(fence_mc_addr);
+   write_frame->cmd_buf_addr_hi = upper_32_bits(cmd_buf_mc_addr);
+   write_frame->cmd_buf_addr_lo = lower_32_bits(cmd_buf_mc_addr);
+   write_frame->fence_addr_hi = upper_32_bits(fence_mc_addr);
+   write_frame->fence_addr_lo = lower_32_bits(fence_mc_addr);
write_frame->fence_value = index;
 
/* Update the write Pointer in DWORDs */
-- 
2.5.5

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

[PATCH] drm/amd/powerplay/cz: print message if smc message fails

[Alex Deucher]

Helpful in debugging.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
index 39c7091..652aaa4 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/cz_smumgr.c
@@ -72,7 +72,7 @@ static int cz_send_msg_to_smc_async(struct pp_smumgr *smumgr,
result = SMUM_WAIT_FIELD_UNEQUAL(smumgr,
SMU_MP1_SRBM2P_RESP_0, CONTENT, 0);
if (result != 0) {
-   pr_err("cz_send_msg_to_smc_async failed\n");
+   pr_err("cz_send_msg_to_smc_async (0x%04x) failed\n", msg);
return result;
}
 
-- 
2.5.5

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

Re: [RFC PATCH linux-next] drm/amdgpu/gfx9: gfx_v9_0_enable_gfx_static_mg_power_gating() can be static

[Alex Deucher]

On Thu, Jun 22, 2017 at 4:28 PM, kbuild test robot
 wrote:
>
> Signed-off-by: Fengguang Wu 

Applied.  thanks!

> ---
>  gfx_v9_0.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index a1e1b7aa..f2ae6f3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1992,8 +1992,8 @@ static void 
> gfx_v9_0_enable_gfx_pipeline_powergating(struct amdgpu_device *adev,
> data = RREG32(SOC15_REG_OFFSET(GC, 0, mmDB_RENDER_CONTROL));
>  }
>
> -void gfx_v9_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
> -   bool enable)
> +static void gfx_v9_0_enable_gfx_static_mg_power_gating(struct amdgpu_device 
> *adev,
> +  bool enable)
>  {
> uint32_t data, default_data;
>
> @@ -2006,7 +2006,7 @@ void gfx_v9_0_enable_gfx_static_mg_power_gating(struct 
> amdgpu_device *adev,
> WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_PG_CNTL), data);
>  }
>
> -void gfx_v9_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
> +static void gfx_v9_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device 
> *adev,
> bool enable)
>  {
> uint32_t data, default_data;
> ___
> 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] Add PCI device based GPU selection with --pci

[Jean-Francois Thibert]

This allows selecting the GPU by its PCI device both with and
without kernel mode support. The instance is populated automatically
so that the proper corresponding debugfs files are used if present.

Signed-off-by: Jean-Francois Thibert 
---
 src/app/main.c |  9 ++
 src/lib/discover.c | 85 --
 src/umr.h  |  6 +++-
 3 files changed, 96 insertions(+), 4 deletions(-)

diff --git a/src/app/main.c b/src/app/main.c
index 1d9ef9e..aa630f5 100644
--- a/src/app/main.c
+++ b/src/app/main.c
@@ -174,6 +174,15 @@ int main(int argc, char **argv)
printf("--force requires a number/name\n");
return EXIT_FAILURE;
}
+   } else if (!strcmp(argv[i], "--pci")) {
+   if (i + 1 < argc && sscanf(argv[i+1], 
"%04x:%02x:%02x.%01x",
+   _domain, _bus, 
_slot,
+   _func ) >= 4) {
+   ++i;
+   } else {
+   printf("--pci requires 
domain:bus:slot.function\n");
+   return EXIT_FAILURE;
+   }
} else if (!strcmp(argv[i], "--print") || !strcmp(argv[i], 
"-p")) {
options.print = 1;
options.need_scan = 1;
diff --git a/src/lib/discover.c b/src/lib/discover.c
index 9662d05..8ab321f 100644
--- a/src/lib/discover.c
+++ b/src/lib/discover.c
@@ -22,6 +22,9 @@
  * Authors: Tom St Denis 
  *
  */
+#include 
+#include 
+
 #include "umr.h"
 
 static int is_did_match(struct umr_asic *asic, unsigned did)
@@ -44,6 +47,43 @@ static int is_did_match(struct umr_asic *asic, unsigned did)
return r;
 }
 
+static int find_pci_instance(const char* pci_string) {
+   DIR* dir;
+   dir = opendir("/sys/kernel/debug/dri");
+   if (dir == NULL) {
+   perror("Couldn't open DRI under debugfs");
+   return -1;
+   }
+   struct dirent *dir_entry;
+   while ((dir_entry = readdir(dir)) != NULL) {
+   // ignore . and ..
+   if (strcmp(dir_entry->d_name, ".") == 0 || 
strcmp(dir_entry->d_name,
+   "..") == 0) {
+   continue;
+   }
+   char name[256];
+   snprintf(name, sizeof(name) - 1, 
"/sys/kernel/debug/dri/%s/name",
+   dir_entry->d_name);
+   FILE *f = fopen(name, "r");
+   if (!f) {
+   continue;
+   }
+   char device[256] = {};
+   int parsed_device = fscanf(f, "%*s %255s", device);
+   fclose(f);
+   if (parsed_device != 1)
+   continue;
+   // strip off dev= for kernels > 4.7
+   if (strstr(device, "dev="))
+   memmove(device, device+4, strlen(device)-3);
+   if (strcmp(pci_string, device) == 0) {
+   closedir(dir);
+   return atoi(dir_entry->d_name);
+   }
+   }
+   closedir(dir);
+   return -1;
+}
 
 struct umr_asic *umr_discover_asic(struct umr_options *options)
 {
@@ -53,6 +93,29 @@ struct umr_asic *umr_discover_asic(struct umr_options 
*options)
struct umr_asic *asic;
long trydid = options->forcedid;
 
+   // Try to map to instance if we have a specific pci device
+   if (options->pci_domain || options->pci_bus ||
+   options->pci_slot || options->pci_func) {
+   char pci_string[16];
+   snprintf(pci_string, sizeof(pci_string) - 1, 
"%04x:%02x:%02x.%x",
+   options->pci_domain, options->pci_bus, 
options->pci_slot,
+   options->pci_func);
+   if (!options->no_kernel) {
+   options->instance = find_pci_instance(pci_string);
+   }
+   snprintf(driver, sizeof(driver)-1, 
"/sys/bus/pci/devices/%s/device", pci_string);
+   f = fopen(driver, "r");
+   if (!f) {
+   if (!options->quiet) perror("Cannot open PCI device 
name under sysfs (is a display attached?)");
+   return NULL;
+   }
+   int found_did = fscanf(f, "0x%04lx", );
+   fclose(f);
+   if (found_did != 1) {
+   if (!options->quiet) printf("Could not read device id");
+   return NULL;
+   }
+   }
// try to scan via debugfs
asic = calloc(1, sizeof *asic);
if (asic) {
@@ -64,7 +127,6 @@ struct umr_asic *umr_discover_asic(struct umr_options 
*options)
umr_free_asic(asic);
asic = NULL;
}
-
if (trydid < 0) {
snprintf(name, 

[RFC PATCH linux-next] drm/amdgpu/gfx9: gfx_v9_0_enable_gfx_static_mg_power_gating() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu 
---
 gfx_v9_0.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index a1e1b7aa..f2ae6f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1992,8 +1992,8 @@ static void 
gfx_v9_0_enable_gfx_pipeline_powergating(struct amdgpu_device *adev,
data = RREG32(SOC15_REG_OFFSET(GC, 0, mmDB_RENDER_CONTROL));
 }
 
-void gfx_v9_0_enable_gfx_static_mg_power_gating(struct amdgpu_device *adev,
-   bool enable)
+static void gfx_v9_0_enable_gfx_static_mg_power_gating(struct amdgpu_device 
*adev,
+  bool enable)
 {
uint32_t data, default_data;
 
@@ -2006,7 +2006,7 @@ void gfx_v9_0_enable_gfx_static_mg_power_gating(struct 
amdgpu_device *adev,
WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_PG_CNTL), data);
 }
 
-void gfx_v9_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device *adev,
+static void gfx_v9_0_enable_gfx_dynamic_mg_power_gating(struct amdgpu_device 
*adev,
bool enable)
 {
uint32_t data, default_data;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[linux-next:master 2278/9292] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1995:6: sparse: symbol 'gfx_v9_0_enable_gfx_static_mg_power_gating' was not declared. Should it be static?

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
master
head:   09c17284731e42dbe4c6d334603e9c05ba1219ae
commit: 18924c719e1d2b194f93ef757584b814421f22a5 [2278/9292] drm/amdgpu/gfx9: 
allow updating gfx mgpg state
reproduce:
# apt-get install sparse
git checkout 18924c719e1d2b194f93ef757584b814421f22a5
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:400:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:402:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:404:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:406:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:408:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:410:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:420:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:422:57: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:427:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:429:53: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:439:36: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:440:41: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:452:17: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:454:17: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:466:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:466:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:466:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:473:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:473:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:473:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:480:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:480:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:480:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:487:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:487:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:487:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:495:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:501:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:501:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:501:25: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:510:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:515:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:515:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:515:33: sparse: cast to restricted 
__le32
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:581:25: sparse: incorrect type in 
assignment (different base types)
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:581:25:expected unsigned int 
volatile [unsigned] [usertype] 
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:581:25:got restricted __le32 
[usertype] 
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:582:25: sparse: incorrect type in 
assignment (different base types)
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:582:25:expected unsigned int 
volatile [unsigned] [usertype] 
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:582:25:got restricted __le32 
[usertype] 
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:584:25: sparse: incorrect type in 
assignment (different base types)
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:584:25:expected unsigned int 
volatile [unsigned] [usertype] 
   

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

[axie]

Hi Marek,

Spin lock and spin unlock is fast. But it is not so fast compared with 
atomic, which is a single CPU instruction in x86.



1. spinlock does NOT allow preemption at local CPU. Let us have a look 
at how spin lock was implemented.


static inline void __raw_spin_lock(raw_spinlock_t *lock)
{
preempt_disable(); --This is 
memory barrier operation too.

spin_acquire(>dep_map, 0, 0, _RET_IP_);
LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
}

2.  A function  __lock_acquire called by spinlock. The function is so 
long that I would not attach all of it here.


There is atomic operation inside and 12 meta data updates and 14 if 
statements and it calls quite some other functions.


Note that it disable IRQ...

static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
  int trylock, int read, int check, int hardirqs_off,
  struct lockdep_map *nest_lock, unsigned long ip,
  int references, int pin_count)
{
struct task_struct *curr = current;
struct lock_class *class = NULL;
struct held_lock *hlock;
unsigned int depth;
int chain_head = 0;
int class_idx;
u64 chain_key;

if (unlikely(!debug_locks))
return 0;

/*
 * Lockdep should run with IRQs disabled, otherwise we could
 * get an interrupt which would want to take locks, which would
 * end up in lockdep and have you got a head-ache already?
 */
if (DEBUG_LOCKS_WARN_ON(!irqs_disabled())) <lockdep_recursion = 1;
trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock, 
ip);

__lock_acquire(lock, subclass, trylock, read, check,
   irqs_disabled_flags(flags), nest_lock, ip, 0, 0);
current->lockdep_recursion = 0;
raw_local_irq_restore(flags);
}


Thanks,

Alex Bin


On 2017-06-22 12:27 PM, Marek Olšák wrote:

On Thu, Jun 22, 2017 at 5:33 PM, Xie, AlexBin  wrote:

Hi Christian,


In fact, the change from spinlock to atomic is quite painful. When I
started, I thought it was easy but later I found there might be race
condition here and there. Now I think the change looks more robust. In
kernel source, there are several other drivers used the same trick.


On the other hand, I think the logic itself might be optimized considering
the locking. But I had spent quite some effort to maintain original logic.

It seems quite complicated and I don't know if there is any
performance benefit. Spinlocks are nice because they allow preemption.

It would be more interesting to merge the CS and BO_LIST ioctls into one.

Marek


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

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

[Marek Olšák]

On Thu, Jun 22, 2017 at 5:33 PM, Xie, AlexBin  wrote:
> Hi Christian,
>
>
> In fact, the change from spinlock to atomic is quite painful. When I
> started, I thought it was easy but later I found there might be race
> condition here and there. Now I think the change looks more robust. In
> kernel source, there are several other drivers used the same trick.
>
>
> On the other hand, I think the logic itself might be optimized considering
> the locking. But I had spent quite some effort to maintain original logic.

It seems quite complicated and I don't know if there is any
performance benefit. Spinlocks are nice because they allow preemption.

It would be more interesting to merge the CS and BO_LIST ioctls into one.

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

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

[Christian König]

Hi Alex,

yeah, the change looks totally ok to me. The problem is just that I'm 
not familiar with that part of the source.


Marek came up with that, so he should at least take a look and nod.

Regards,
Christian.

Am 22.06.2017 um 17:33 schrieb Xie, AlexBin:


Hi Christian,


In fact, the change from spinlock to atomic is quite painful. When I 
started, I thought it was easy but later I found there might be race 
condition here and there. Now I think the change looks more robust. In 
kernel source, there are several other drivers used the same trick.



On the other hand, I think the logic itself might be optimized 
considering the locking. But I had spent quite some effort to maintain 
original logic.



Thanks,

Alex Bin


*From:* Christian König 
*Sent:* Thursday, June 22, 2017 3:35 AM
*To:* Xie, AlexBin; amd-gfx@lists.freedesktop.org
*Subject:* Re: [PATCH 1/3] drm/amdgpu: fix a typo
Am 22.06.2017 um 04:42 schrieb Alex Xie:
> Signed-off-by: Alex Xie 

With the commit message fixed as Michel suggested patches #1 and #2 are
Reviewed-by: Christian König  as well.

On patch #3 Marek needs to take a look, cause I don't know the logic
behind that.

Regards,
Christian.

> ---
>   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,

> >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,




___
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 1/3] drm/amdgpu: fix a typo

[Xie, AlexBin]

Hi Christian,


In fact, the change from spinlock to atomic is quite painful. When I started, I 
thought it was easy but later I found there might be race condition here and 
there. Now I think the change looks more robust. In kernel source, there are 
several other drivers used the same trick.


On the other hand, I think the logic itself might be optimized considering the 
locking. But I had spent quite some effort to maintain original logic.


Thanks,

Alex Bin


From: Christian König 
Sent: Thursday, June 22, 2017 3:35 AM
To: Xie, AlexBin; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/amdgpu: fix a typo

Am 22.06.2017 um 04:42 schrieb Alex Xie:
> Signed-off-by: Alex Xie 

With the commit message fixed as Michel suggested patches #1 and #2 are
Reviewed-by: Christian König  as well.

On patch #3 Marek needs to take a look, cause I don't know the logic
behind that.

Regards,
Christian.

> ---
>   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,
> >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,


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

RE: [PATCH umr] Fix copy/paste error in error message

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Thursday, June 22, 2017 9:38 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH umr] Fix copy/paste error in error message
> 
> Also fix a couple of error message style deviations.
> 
> Signed-off-by: Tom St Denis 

Reviewed-by: Alex Deucher 

> ---
>  src/lib/mmio.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lib/mmio.c b/src/lib/mmio.c
> index f0737c43913a..987704cb9944 100644
> --- a/src/lib/mmio.c
> +++ b/src/lib/mmio.c
> @@ -39,7 +39,7 @@ static uint32_t umr_smc_read(struct umr_asic *asic,
> uint64_t addr)
>   umr_write_reg_by_name(asic,
> "mmMP0PUB_IND_INDEX_1", addr);
>   return umr_read_reg_by_name(asic,
> "mmMP0PUB_IND_DATA_1");
>   default:
> - fprintf(stderr, "[BUG] Unsupported family
> type in umr_smc_read()\n");
> + fprintf(stderr, "[BUG]: Unsupported family
> type in umr_smc_read()\n");
>   return 0;
>   }
>   } else {
> @@ -66,7 +66,7 @@ static uint32_t umr_smc_write(struct umr_asic *asic,
> uint64_t addr, uint32_t val
>   umr_write_reg_by_name(asic,
> "mmMP0PUB_IND_INDEX_1", addr);
>   return umr_write_reg_by_name(asic,
> "mmMP0PUB_IND_DATA_1", value);
>   default:
> - fprintf(stderr, "[BUG] Unsupported family
> type in umr_smc_read()\n");
> + fprintf(stderr, "[BUG]: Unsupported family
> type in umr_smc_write()\n");
>   return -1;
>   }
>   } else {
> @@ -103,7 +103,7 @@ uint32_t umr_read_reg(struct umr_asic *asic,
> uint64_t addr, enum regclass type)
>   case REG_SMC:
>   return umr_smc_read(asic, addr);
>   default:
> - fprintf(stderr, "[BUG] Unsupported register type in
> umr_read_reg().\n");
> + fprintf(stderr, "[BUG]: Unsupported register type in
> umr_read_reg().\n");
>   return 0;
>   }
>  }
> @@ -131,7 +131,7 @@ int umr_write_reg(struct umr_asic *asic, uint64_t
> addr, uint32_t value, enum reg
>   case REG_SMC:
>   return umr_smc_write(asic, addr, value);
>   default:
> - fprintf(stderr, "[BUG] Unsupported register type in
> umr_write_reg().\n");
> + fprintf(stderr, "[BUG]: Unsupported register type in
> umr_write_reg().\n");
>   return -1;
>   }
>   return 0;
> --
> 2.12.0
> 
> ___
> 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

[PATCH umr] Fix copy/paste error in error message

[Tom St Denis]

Also fix a couple of error message style deviations.

Signed-off-by: Tom St Denis 
---
 src/lib/mmio.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lib/mmio.c b/src/lib/mmio.c
index f0737c43913a..987704cb9944 100644
--- a/src/lib/mmio.c
+++ b/src/lib/mmio.c
@@ -39,7 +39,7 @@ static uint32_t umr_smc_read(struct umr_asic *asic, uint64_t 
addr)
umr_write_reg_by_name(asic, 
"mmMP0PUB_IND_INDEX_1", addr);
return umr_read_reg_by_name(asic, 
"mmMP0PUB_IND_DATA_1");
default:
-   fprintf(stderr, "[BUG] Unsupported family type 
in umr_smc_read()\n");
+   fprintf(stderr, "[BUG]: Unsupported family type 
in umr_smc_read()\n");
return 0;
}
} else {
@@ -66,7 +66,7 @@ static uint32_t umr_smc_write(struct umr_asic *asic, uint64_t 
addr, uint32_t val
umr_write_reg_by_name(asic, 
"mmMP0PUB_IND_INDEX_1", addr);
return umr_write_reg_by_name(asic, 
"mmMP0PUB_IND_DATA_1", value);
default:
-   fprintf(stderr, "[BUG] Unsupported family type 
in umr_smc_read()\n");
+   fprintf(stderr, "[BUG]: Unsupported family type 
in umr_smc_write()\n");
return -1;
}
} else {
@@ -103,7 +103,7 @@ uint32_t umr_read_reg(struct umr_asic *asic, uint64_t addr, 
enum regclass type)
case REG_SMC:
return umr_smc_read(asic, addr);
default:
-   fprintf(stderr, "[BUG] Unsupported register type in 
umr_read_reg().\n");
+   fprintf(stderr, "[BUG]: Unsupported register type in 
umr_read_reg().\n");
return 0;
}
 }
@@ -131,7 +131,7 @@ int umr_write_reg(struct umr_asic *asic, uint64_t addr, 
uint32_t value, enum reg
case REG_SMC:
return umr_smc_write(asic, addr, value);
default:
-   fprintf(stderr, "[BUG] Unsupported register type in 
umr_write_reg().\n");
+   fprintf(stderr, "[BUG]: Unsupported register type in 
umr_write_reg().\n");
return -1;
}
return 0;
-- 
2.12.0

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

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

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Wednesday, June 21, 2017 11:56 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH xf86-video-amdgpu] Increase reference count of FB
> assigned to drmmode_crtc->flip_pending
> 
> 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 

Reviewed-by: Alex Deucher 

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

RE: [PATCH xf86-video-amdgpu] Improve drmmode_fb_reference debugging code

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Thursday, June 22, 2017 4:03 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH xf86-video-amdgpu] Improve drmmode_fb_reference
> debugging code
> 
> From: Michel Dänzer 
> 
> If a reference count is <= 0, call FatalError with the call location
> (in case it doesn't get resolved in the backtrace printed by
> FatalError).
> 
> Signed-off-by: Michel Dänzer 

Reviewed-by: Alex Deucher 

> ---
>  src/drmmode_display.h | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/src/drmmode_display.h b/src/drmmode_display.h
> index b64a938cd..f351bb09c 100644
> --- a/src/drmmode_display.h
> +++ b/src/drmmode_display.h
> @@ -133,32 +133,36 @@ enum drmmode_flip_sync {
> 
> 
>  static inline void
> -drmmode_fb_reference(int drm_fd, struct drmmode_fb **old, struct
> drmmode_fb *new)
> +drmmode_fb_reference_loc(int drm_fd, struct drmmode_fb **old, struct
> drmmode_fb *new,
> +  const char *caller, unsigned line)
>  {
>   if (new) {
>   if (new->refcnt <= 0) {
> - ErrorF("New FB's refcnt was %d in %s\n", new-
> >refcnt,
> -__func__);
> - } else {
> - new->refcnt++;
> + FatalError("New FB's refcnt was %d at %s:%u",
> +new->refcnt, caller, line);
>   }
> +
> + new->refcnt++;
>   }
> 
>   if (*old) {
>   if ((*old)->refcnt <= 0) {
> - ErrorF("Old FB's refcnt was %d in %s\n",
> -(*old)->refcnt, __func__);
> - } else {
> - if (--(*old)->refcnt == 0) {
> - drmModeRmFB(drm_fd, (*old)->handle);
> - free(*old);
> - }
> + FatalError("Old FB's refcnt was %d at %s:%u",
> +(*old)->refcnt, caller, line);
> + }
> +
> + if (--(*old)->refcnt == 0) {
> + drmModeRmFB(drm_fd, (*old)->handle);
> + free(*old);
>   }
>   }
> 
>   *old = new;
>  }
> 
> +#define drmmode_fb_reference(fd, old, new) \
> + drmmode_fb_reference_loc(fd, old, new, __func__, __LINE__)
> +
> 
>  extern int drmmode_page_flip_target_absolute(AMDGPUEntPtr
> pAMDGPUEnt,
>drmmode_crtc_private_ptr
> drmmode_crtc,
> --
> 2.11.0
> 
> ___
> 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

[PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap

[Peter Rosin]

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?

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +++
 1 file changed, 27 insertions(+)

This is an alternative version rebased on top of Daniels "fbdev helper
locking rework and deferred setup" series.

Cheers,
peda

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a4cfef9..c7122c9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1330,12 +1330,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
const struct drm_crtc_helper_funcs *crtc_funcs;
u16 *red, *green, *blue, *transp;
struct drm_crtc *crtc;
+   u16 *r, *g, *b;
int i, j, rc = 0;
int start;
 
if (oops_in_progress)
return -EBUSY;
 
+   if (cmap->start + cmap->len < cmap->start)
+   return -EINVAL;
+
mutex_lock(_helper->lock);
if (!drm_fb_helper_is_bound(fb_helper)) {
mutex_unlock(_helper->lock);
@@ -1353,6 +1357,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
transp = cmap->transp;
start = cmap->start;
 
+   if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+   if (!crtc->gamma_size) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   if (cmap->start + cmap->len > crtc->gamma_size) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
+
+   memcpy(r + cmap->start, cmap->red,
+  cmap->len * sizeof(u16));
+   memcpy(g + cmap->start, cmap->green,
+  cmap->len * sizeof(u16));
+   memcpy(b + cmap->start, cmap->blue,
+  cmap->len * sizeof(u16));
+   }
+
for (j = 0; j < cmap->len; j++) {
u16 hred, hgreen, hblue, htransp = 0x;
 
-- 
2.1.4


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

[PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_fb_helper.c | 151 +---
 1 file changed, 63 insertions(+), 88 deletions(-)

This is an alternative version rebased on top of Daniel's "fbdev helper
locking rework and deferred setup" series.

And as noted by Daniel, .gamma_set does an atomic commit. Thus, the locks
needs to be dropped and reacquired for each crtc. So, that is fixed here
too. Doing it like this with a couple of individual alternative patches
instead of sending a whole new series since the dependency on Daniel's
series makes life somewhat difficult...

Cheers,
peda

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4aceb59..aa025f1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1257,50 +1257,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) {
-   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 _ops.fb_setcmap
  * @cmap: cmap to set
@@ -1310,12 +1266,10 @@ 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;
u16 *r, *g, *b;
-   int i, j, rc = 0;
-   int start;
+   int i, ret = 0;
 
if (oops_in_progress)
return -EBUSY;
@@ -1329,61 +1283,82 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
return -EBUSY;
}
 
-   drm_modeset_lock_all(dev);
-   for (i = 0; i < fb_helper->crtc_count; i++) {
-   crtc = fb_helper->crtc_info[i].mode_set.crtc;
-   crtc_funcs = crtc->helper_private;
+   drm_modeset_acquire_init(, 0);
 
-   red = cmap->red;
-   green = cmap->green;
-   blue = cmap->blue;
-   transp = cmap->transp;
-   start = cmap->start;
+   for (i = 0; i < fb_helper->crtc_count; i++) {
+   if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+   u32 *palette;
+   int j;
 
-   if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
-   if (!crtc->gamma_size) {
-   rc = -EINVAL;
-   goto out;
+   if (cmap->start + cmap->len > 16) {
+   ret = -EINVAL;
+   break;
}
 
-   if (cmap->start + cmap->len > crtc->gamma_size) {
-   rc = -EINVAL;
-   goto out;
+   palette = (u32 *)info->pseudo_palette;
+   for (j = 0; j < cmap->len; ++j) {
+   u16 red = cmap->red[j];
+   u16 green = cmap->green[j];
+   u16 blue = cmap->blue[j];
+   u32 value;
+
+   red >>= 16 - info->var.red.length;
+ 

[PATCH v2 09/14] drm: i915: remove dead code and pointless local lut storage

[Peter Rosin]

The driver stores lut values from the fbdev interface, and is able
to give them back, but does not appear to do anything with these
lut values. The generic fb helpers have replaced this function,
and may even have made the driver work for the C8 mode from the
fbdev interface. But that is untested.

Since the fb helpers .gamma_set and .gamma_get are obsolete,
remove the dead code.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/i915/intel_drv.h   |  1 -
 drivers/gpu/drm/i915/intel_fbdev.c | 31 ---
 2 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d93efb4..bc7bfa0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -786,7 +786,6 @@ struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
enum plane plane;
-   u8 lut_r[256], lut_g[256], lut_b[256];
/*
 * Whether the crtc and the connected output pipeline is active. Implies
 * that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6..5bac953 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -281,27 +281,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
return ret;
 }
 
-/** Sets the color ramps on behalf of RandR */
-static void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-   u16 blue, int regno)
-{
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-   intel_crtc->lut_r[regno] = red >> 8;
-   intel_crtc->lut_g[regno] = green >> 8;
-   intel_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 
*green,
-   u16 *blue, int regno)
-{
-   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-   *red = intel_crtc->lut_r[regno] << 8;
-   *green = intel_crtc->lut_g[regno] << 8;
-   *blue = intel_crtc->lut_b[regno] << 8;
-}
-
 static struct drm_fb_helper_crtc *
 intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
 {
@@ -370,7 +349,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_fb_helper_crtc *new_crtc;
-   struct intel_crtc *intel_crtc;
 
fb_conn = fb_helper->connector_info[i];
connector = fb_conn->connector;
@@ -412,13 +390,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
 
num_connectors_enabled++;
 
-   intel_crtc = to_intel_crtc(connector->state->crtc);
-   for (j = 0; j < 256; j++) {
-   intel_crtc->lut_r[j] = j;
-   intel_crtc->lut_g[j] = j;
-   intel_crtc->lut_b[j] = j;
-   }
-
new_crtc = intel_fb_helper_crtc(fb_helper,
connector->state->crtc);
 
@@ -519,8 +490,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
 
 static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.initial_config = intel_fb_initial_config,
-   .gamma_set = intel_crtc_fb_gamma_set,
-   .gamma_get = intel_crtc_fb_gamma_get,
.fb_probe = intelfb_create,
 };
 
-- 
2.1.4

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

[PATCH v2 12/14] drm: radeon: remove dead code and pointless local lut storage

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 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/radeon/radeon_mode.h|  4 --
 6 files changed, 33 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c 
b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3c492a0..02baaaf 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -2217,7 +2217,6 @@ static const struct drm_crtc_helper_funcs 
atombios_helper_funcs = {
.mode_set_base_atomic = atombios_crtc_set_base_atomic,
.prepare = atombios_crtc_prepare,
.commit = atombios_crtc_commit,
-   .load_lut = radeon_crtc_load_lut,
.disable = atombios_crtc_disable,
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbd..2f642cb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -773,12 +773,15 @@ static int radeon_connector_set_property(struct 
drm_connector *connector, struct
 
if (connector->encoder->crtc) {
struct drm_crtc *crtc  = connector->encoder->crtc;
-   const struct drm_crtc_helper_funcs *crtc_funcs = 
crtc->helper_private;
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 
radeon_crtc->output_csc = radeon_encoder->output_csc;
 
-   (*crtc_funcs->load_lut)(crtc);
+   /*
+* Our .gamma_set assumes the .gamma_store has been
+* prefilled and don't care about its arguments.
+*/
+   crtc->funcs->gamma_set(crtc, NULL, NULL, NULL, 0, NULL);
}
}
 
diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..8b7d7a0 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+   u16 *r, *g, *b;
int i;
 
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x003f);
 
WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(AVIVO_DC_LUT_30_COLOR,
-(radeon_crtc->lut_r[i] << 20) |
-(radeon_crtc->lut_g[i] << 10) |
-(radeon_crtc->lut_b[i] << 0));
+  ((*r++ & 0xffc0) << 14) |
+  ((*g++ & 0xffc0) << 4) |
+  (*b++ >> 6));
}
 
/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+   u16 *r, *g, *b;
int i;
 
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 
0x0007);
 
WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
-  (radeon_crtc->lut_r[i] << 20) |
-  (radeon_crtc->lut_g[i] << 10) |
-  (radeon_crtc->lut_b[i] << 0));
+  ((*r++ & 0xffc0) << 14) |
+  ((*g++ & 0xffc0) << 4) |
+  (*b++ >> 6));
}
 }
 
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+   u16 *r, *g, *b;
int i;
 
DRM_DEBUG_KMS("%d\n", 

[PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap

[Peter Rosin]

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?

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_fb_helper.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 574af01..25315fb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
const struct drm_crtc_helper_funcs *crtc_funcs;
u16 *red, *green, *blue, *transp;
struct drm_crtc *crtc;
+   u16 *r, *g, *b;
int i, j, rc = 0;
int start;
 
if (oops_in_progress)
return -EBUSY;
 
+   if (cmap->start + cmap->len < cmap->start)
+   return -EINVAL;
+
drm_modeset_lock_all(dev);
if (!drm_fb_helper_is_bound(fb_helper)) {
drm_modeset_unlock_all(dev);
@@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
transp = cmap->transp;
start = cmap->start;
 
+   if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+   if (!crtc->gamma_size) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   if (cmap->start + cmap->len > crtc->gamma_size) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
+
+   memcpy(r + cmap->start, cmap->red,
+  cmap->len * sizeof(u16));
+   memcpy(g + cmap->start, cmap->green,
+  cmap->len * sizeof(u16));
+   memcpy(b + cmap->start, cmap->blue,
+  cmap->len * sizeof(u16));
+   }
+
for (j = 0; j < cmap->len; j++) {
u16 hred, hgreen, hblue, htransp = 0x;
 
-- 
2.1.4

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

[PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set

[Peter Rosin]

This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_fb_helper.c | 154 
 1 file changed, 63 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7ade384..58eb045 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1150,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) {
-   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 _ops.fb_setcmap
  * @cmap: cmap to set
@@ -1203,12 +1159,10 @@ 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;
u16 *r, *g, *b;
-   int i, j, rc = 0;
-   int start;
+   int i, ret;
 
if (oops_in_progress)
return -EBUSY;
@@ -1216,65 +1170,83 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
fb_info *info)
if (cmap->start + cmap->len < cmap->start)
return -EINVAL;
 
-   drm_modeset_lock_all(dev);
+   drm_modeset_acquire_init(, 0);
+retry:
+   ret = drm_modeset_lock_all_ctx(dev, );
+   if (ret)
+   goto out;
if (!drm_fb_helper_is_bound(fb_helper)) {
-   drm_modeset_unlock_all(dev);
-   return -EBUSY;
+   ret = -EBUSY;
+   goto out;
}
 
for (i = 0; i < fb_helper->crtc_count; i++) {
-   crtc = fb_helper->crtc_info[i].mode_set.crtc;
-   crtc_funcs = crtc->helper_private;
-
-   red = cmap->red;
-   green = cmap->green;
-   blue = cmap->blue;
-   transp = cmap->transp;
-   start = cmap->start;
+   if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+   u32 *palette;
+   int j;
 
-   if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
-   if (!crtc->gamma_size) {
-   rc = -EINVAL;
+   if (cmap->start + cmap->len > 16) {
+   ret = -EINVAL;
goto out;
}
 
-   if (cmap->start + cmap->len > crtc->gamma_size) {
-   rc = -EINVAL;
-   goto out;
+   palette = (u32 *)info->pseudo_palette;
+   for (j = 0; j < cmap->len; ++j) {
+   u16 red = cmap->red[j];
+   u16 green = cmap->green[j];
+   u16 blue = cmap->blue[j];
+   u32 value;
+
+   red >>= 16 - info->var.red.length;
+   green >>= 16 - info->var.green.length;
+   blue >>= 16 - info->var.blue.length;
+   value = (red << info->var.red.offset) |
+ 

[PATCH v2 06/14] drm: ast: remove dead code and pointless local lut storage

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 -
 drivers/gpu/drm/ast/ast_fb.c   | 20 
 drivers/gpu/drm/ast/ast_mode.c | 26 ++
 3 files changed, 6 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 8880f0b..569a148 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -245,7 +245,6 @@ struct ast_connector {
 
 struct ast_crtc {
struct drm_crtc base;
-   u8 lut_r[256], lut_g[256], lut_b[256];
struct drm_gem_object *cursor_bo;
uint64_t cursor_addr;
int cursor_width, cursor_height;
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 4ad4acd..dbabcac 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -255,27 +255,7 @@ static int astfb_create(struct drm_fb_helper *helper,
return ret;
 }
 
-static void ast_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-  u16 blue, int regno)
-{
-   struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-   ast_crtc->lut_r[regno] = red >> 8;
-   ast_crtc->lut_g[regno] = green >> 8;
-   ast_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void ast_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-  u16 *blue, int regno)
-{
-   struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-   *red = ast_crtc->lut_r[regno] << 8;
-   *green = ast_crtc->lut_g[regno] << 8;
-   *blue = ast_crtc->lut_b[regno] << 8;
-}
-
 static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
-   .gamma_set = ast_fb_gamma_set,
-   .gamma_get = ast_fb_gamma_get,
.fb_probe = astfb_create,
 };
 
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index aaef0a6..724c16b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -63,15 +63,18 @@ static inline void ast_load_palette_index(struct 
ast_private *ast,
 static void ast_crtc_load_lut(struct drm_crtc *crtc)
 {
struct ast_private *ast = crtc->dev->dev_private;
-   struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+   u16 *r, *g, *b;
int i;
 
if (!crtc->enabled)
return;
 
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++)
-   ast_load_palette_index(ast, i, ast_crtc->lut_r[i],
-  ast_crtc->lut_g[i], ast_crtc->lut_b[i]);
+   ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
 }
 
 static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct 
drm_display_mode *mode,
@@ -633,7 +636,6 @@ static const struct drm_crtc_helper_funcs 
ast_crtc_helper_funcs = {
.mode_set = ast_crtc_mode_set,
.mode_set_base = ast_crtc_mode_set_base,
.disable = ast_crtc_disable,
-   .load_lut = ast_crtc_load_lut,
.prepare = ast_crtc_prepare,
.commit = ast_crtc_commit,
 
@@ -648,15 +650,6 @@ static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 
*red, u16 *green,
  u16 *blue, uint32_t size,
  struct drm_modeset_acquire_ctx *ctx)
 {
-   struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-   int i;
-
-   /* userspace palettes are always correct as is */
-   for (i = 0; i < size; i++) {
-   ast_crtc->lut_r[i] = red[i] >> 8;
-   ast_crtc->lut_g[i] = green[i] >> 8;
-   ast_crtc->lut_b[i] = blue[i] >> 8;
-   }
ast_crtc_load_lut(crtc);
 
return 0;
@@ -681,7 +674,6 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
 static int ast_crtc_init(struct drm_device *dev)
 {
struct ast_crtc *crtc;
-   int i;
 
crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
if (!crtc)
@@ -690,12 +682,6 @@ static int ast_crtc_init(struct drm_device *dev)
drm_crtc_init(dev, >base, _crtc_funcs);
drm_mode_crtc_set_gamma_size(>base, 256);
drm_crtc_helper_add(>base, _crtc_helper_funcs);
-
-   for (i = 0; i < 256; i++) {
-   crtc->lut_r[i] = i;
-   crtc->lut_g[i] = i;
-   crtc->lut_b[i] = i;
-   }
return 0;
 }
 
-- 
2.1.4

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

[PATCH v2 02/14] drm/fb-helper: remove drm_fb_helper_save_lut_atomic

[Peter Rosin]

drm_fb_helper_save_lut_atomic is redundant since the .gamma_store is
now always kept up to date by drm_fb_helper_setcmap.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/drm_fb_helper.c | 17 -
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 25315fb..7ade384 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, _base[i], _base[i], 
_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,
-- 
2.1.4

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

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

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 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  | 40 +++--
 4 files changed, 22 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..8f689f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
struct drm_device *dev = nv_crtc->base.dev;
struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+   u16 *r, *g, *b;
int i;
 
rgbs = (struct rgb 
*)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++) {
-   rgbs[i].r = nv_crtc->lut.r[i] >> 8;
-   rgbs[i].g = nv_crtc->lut.g[i] >> 8;
-   rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+   rgbs[i].r = *r++ >> 8;
+   rgbs[i].g = *g++ >> 8;
+   rgbs[i].b = *b++ >> 8;
}
 
nouveau_hw_load_state_palette(dev, nv_crtc->index, 
_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, 
u16 *b,
  struct drm_modeset_acquire_ctx *ctx)
 {
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-   int i;
-
-   for (i = 0; i < size; i++) {
-   nv_crtc->lut.r[i] = r[i];
-   nv_crtc->lut.g[i] = g[i];
-   nv_crtc->lut.b[i] = b[i];
-   }
 
/* We need to know the depth before we upload, but it's possible to
 * get called before a framebuffer is bound.  If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs 
nv04_crtc_helper_funcs = {
.mode_set = nv_crtc_mode_set,
.mode_set_base = nv04_crtc_mode_set_base,
.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
-   .load_lut = nv_crtc_gamma_load,
.disable = nv_crtc_disable,
 };
 
@@ -1103,17 +1100,12 @@ int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
struct nouveau_crtc *nv_crtc;
-   int ret, i;
+   int ret;
 
nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
if (!nv_crtc)
return -ENOMEM;
 
-   for (i = 0; i < 256; i++) {
-   nv_crtc->lut.r[i] = i << 8;
-   nv_crtc->lut.g[i] = i << 8;
-   nv_crtc->lut.b[i] = i << 8;
-   }
nv_crtc->lut.depth = 0;
 
nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h 
b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf3..b7a18fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -61,9 +61,6 @@ struct nouveau_crtc {
 
struct {
struct nouveau_bo *nvbo;
-   uint16_t r[256];
-   uint16_t g[256];
-   uint16_t b[256];
int depth;
} lut;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c 
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2665a07..f770784 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -278,26 +278,6 @@ nouveau_fbcon_accel_init(struct drm_device *dev)
info->fbops = _fbcon_ops;
 }
 
-static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-   u16 blue, int regno)
-{
-   struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
-   nv_crtc->lut.r[regno] = red;
-   nv_crtc->lut.g[regno] = green;
-   nv_crtc->lut.b[regno] = blue;
-}
-
-static void nouveau_fbcon_gamma_get(struct drm_crtc *crtc, u16 *red, u16 
*green,
-   u16 *blue, int regno)
-{
-   struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
-   *red = nv_crtc->lut.r[regno];
-   *green = nv_crtc->lut.g[regno];
-   *blue = nv_crtc->lut.b[regno];
-}
-
 static void
 nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon)
 {
@@ -467,8 +447,6 @@ void nouveau_fbcon_gpu_lockup(struct fb_info *info)
 }
 
 static const struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
-   .gamma_set = nouveau_fbcon_gamma_set,
-   .gamma_get = nouveau_fbcon_gamma_get,
.fb_probe = nouveau_fbcon_create,
 };
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 

[PATCH v2 04/14] drm: amd: remove dead code and pointless local lut storage

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 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 ---
 7 files changed, 28 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index c0d8c6f..7dc3780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -312,31 +312,7 @@ static int amdgpu_fbdev_destroy(struct drm_device *dev, 
struct amdgpu_fbdev *rfb
return 0;
 }
 
-/** Sets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
-   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-   amdgpu_crtc->lut_r[regno] = red >> 6;
-   amdgpu_crtc->lut_g[regno] = green >> 6;
-   amdgpu_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 
*green,
- u16 *blue, int regno)
-{
-   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
-   *red = amdgpu_crtc->lut_r[regno] << 6;
-   *green = amdgpu_crtc->lut_g[regno] << 6;
-   *blue = amdgpu_crtc->lut_b[regno] << 6;
-}
-
 static const struct drm_fb_helper_funcs amdgpu_fb_helper_funcs = {
-   .gamma_set = amdgpu_crtc_fb_gamma_set,
-   .gamma_get = amdgpu_crtc_fb_gamma_get,
.fb_probe = amdgpufb_create,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 43a9d3a..39f7eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -369,7 +369,6 @@ struct amdgpu_atom_ss {
 struct amdgpu_crtc {
struct drm_crtc base;
int crtc_id;
-   u16 lut_r[256], lut_g[256], lut_b[256];
bool enabled;
bool can_tile;
uint32_t crtc_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 9f78c03..c958023 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct amdgpu_device *adev = dev->dev_private;
+   u16 *r, *g, *b;
int i;
u32 tmp;
 
@@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc 
*crtc)
WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x0007);
 
WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
-  (amdgpu_crtc->lut_r[i] << 20) |
-  (amdgpu_crtc->lut_g[i] << 10) |
-  (amdgpu_crtc->lut_b[i] << 0));
+  ((*r++ & 0xffc0) << 14) |
+  ((*g++ & 0xffc0) << 4) |
+  (*b++ >> 6));
}
 
tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc 
*crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
 {
-   struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-   int i;
-
-   /* userspace palettes are always correct as is */
-   for (i = 0; i < size; i++) {
-   amdgpu_crtc->lut_r[i] = red[i] >> 6;
-   amdgpu_crtc->lut_g[i] = green[i] >> 6;
-   amdgpu_crtc->lut_b[i] = blue[i] >> 6;
-   }
dce_v10_0_crtc_load_lut(crtc);
 
return 0;
@@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs 
dce_v10_0_crtc_helper_funcs = {
.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
.prepare = dce_v10_0_crtc_prepare,
.commit = dce_v10_0_crtc_commit,
-   .load_lut = dce_v10_0_crtc_load_lut,
.disable = dce_v10_0_crtc_disable,
 };
 
 static int dce_v10_0_crtc_init(struct 

[PATCH v2 05/14] drm: armada: remove dead empty functions

[Peter Rosin]

The redundant fb helpers .gamma_set and .gamma_get are no longer used.
Remove the dead code.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/armada/armada_crtc.c  | 10 --
 drivers/gpu/drm/armada/armada_crtc.h  |  2 --
 drivers/gpu/drm/armada/armada_fbdev.c |  2 --
 3 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_crtc.c 
b/drivers/gpu/drm/armada/armada_crtc.c
index 4fe19fd..96bccf8 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -334,16 +334,6 @@ static void armada_drm_vblank_off(struct armada_crtc 
*dcrtc)
armada_drm_plane_work_run(dcrtc, dcrtc->crtc.primary);
 }
 
-void armada_drm_crtc_gamma_set(struct drm_crtc *crtc, u16 r, u16 g, u16 b,
-   int idx)
-{
-}
-
-void armada_drm_crtc_gamma_get(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-   int idx)
-{
-}
-
 /* The mode_config.mutex will be held for this call */
 static void armada_drm_crtc_dpms(struct drm_crtc *crtc, int dpms)
 {
diff --git a/drivers/gpu/drm/armada/armada_crtc.h 
b/drivers/gpu/drm/armada/armada_crtc.h
index 7e8906d..bab11f4 100644
--- a/drivers/gpu/drm/armada/armada_crtc.h
+++ b/drivers/gpu/drm/armada/armada_crtc.h
@@ -102,8 +102,6 @@ struct armada_crtc {
 };
 #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
 
-void armada_drm_crtc_gamma_set(struct drm_crtc *, u16, u16, u16, int);
-void armada_drm_crtc_gamma_get(struct drm_crtc *, u16 *, u16 *, u16 *, int);
 void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *);
 
 void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc,
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c 
b/drivers/gpu/drm/armada/armada_fbdev.c
index 602dfea..5fa076d 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -118,8 +118,6 @@ static int armada_fb_probe(struct drm_fb_helper *fbh,
 }
 
 static const struct drm_fb_helper_funcs armada_fb_helper_funcs = {
-   .gamma_set  = armada_drm_crtc_gamma_set,
-   .gamma_get  = armada_drm_crtc_gamma_get,
.fb_probe   = armada_fb_probe,
 };
 
-- 
2.1.4

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

[PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Peter Rosin]

The redundant fb helper .load_lut is no longer used, and can not
work right without also providing the fb helpers .gamma_set and
.gamma_get thus rendering the code in this driver suspect.

Just remove the dead code.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/stm/ltdc.c | 12 
 drivers/gpu/drm/stm/ltdc.h |  1 -
 2 files changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 1b9483d..87829b9 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
  * DRM_CRTC
  */
 
-static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
-{
-   struct ltdc_device *ldev = crtc_to_ltdc(crtc);
-   unsigned int i, lay;
-
-   for (lay = 0; lay < ldev->caps.nb_layers; lay++)
-   for (i = 0; i < 256; i++)
-   reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
- ldev->clut[i]);
-}
-
 static void ltdc_crtc_enable(struct drm_crtc *crtc)
 {
struct ltdc_device *ldev = crtc_to_ltdc(crtc);
@@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
 }
 
 static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
-   .load_lut = ltdc_crtc_load_lut,
.enable = ltdc_crtc_enable,
.disable = ltdc_crtc_disable,
.mode_set_nofb = ltdc_crtc_mode_set_nofb,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c73..620ca55 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -27,7 +27,6 @@ struct ltdc_device {
struct drm_panel *panel;
struct mutex err_lock;  /* protecting error_status */
struct ltdc_caps caps;
-   u32 clut[256];  /* color look up table */
u32 error_status;
u32 irq_status;
 };
-- 
2.1.4

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

[PATCH v2 07/14] drm: cirrus: remove dead code and pointless local lut storage

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/cirrus/cirrus_drv.h   |  8 
 drivers/gpu/drm/cirrus/cirrus_fbdev.c |  2 -
 drivers/gpu/drm/cirrus/cirrus_mode.c  | 71 ---
 3 files changed, 16 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h 
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 8690352..be2d7e48 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -96,7 +96,6 @@
 
 struct cirrus_crtc {
struct drm_crtc base;
-   u8  lut_r[256], lut_g[256], lut_b[256];
int last_dpms;
boolenabled;
 };
@@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo)
 #define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
 #define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT)
 
-   /* cirrus_mode.c */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
-
-
/* cirrus_main.c */
 int cirrus_device_init(struct cirrus_device *cdev,
  struct drm_device *ddev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58ee..1fedab0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
 }
 
 static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
-   .gamma_set = cirrus_crtc_fb_gamma_set,
-   .gamma_get = cirrus_crtc_fb_gamma_get,
.fb_probe = cirrusfb_create,
 };
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c 
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..a4c4a46 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
  * This file contains setup code for the CRTC.
  */
 
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
-   struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-   struct drm_device *dev = crtc->dev;
-   struct cirrus_device *cdev = dev->dev_private;
-   int i;
-
-   if (!crtc->enabled)
-   return;
-
-   for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
-   /* VGA registers */
-   WREG8(PALETTE_INDEX, i);
-   WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
-   WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
-   WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
-   }
-}
-
 /*
  * The DRM core requires DPMS functions, but they make little sense in our
  * case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, 
u16 *red, u16 *green,
 u16 *blue, uint32_t size,
 struct drm_modeset_acquire_ctx *ctx)
 {
-   struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+   struct drm_device *dev = crtc->dev;
+   struct cirrus_device *cdev = dev->dev_private;
+   u16 *r, *g, *b;
int i;
 
-   for (i = 0; i < size; i++) {
-   cirrus_crtc->lut_r[i] = red[i];
-   cirrus_crtc->lut_g[i] = green[i];
-   cirrus_crtc->lut_b[i] = blue[i];
+   if (!crtc->enabled)
+   return 0;
+
+   r = crtc->gamma_store;
+   g = r + crtc->gamma_size;
+   b = g + crtc->gamma_size;
+
+   for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+   /* VGA registers */
+   WREG8(PALETTE_INDEX, i);
+   WREG8(PALETTE_DATA, *r++ >> 8);
+   WREG8(PALETTE_DATA, *g++ >> 8);
+   WREG8(PALETTE_DATA, *b++ >> 8);
}
-   cirrus_crtc_load_lut(crtc);
 
return 0;
 }
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs 
cirrus_helper_funcs = {
.mode_set_base = cirrus_crtc_mode_set_base,
.prepare = cirrus_crtc_prepare,
.commit = cirrus_crtc_commit,
-   .load_lut = cirrus_crtc_load_lut,
 };
 
 /* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
 {
struct cirrus_device *cdev = dev->dev_private;
struct cirrus_crtc *cirrus_crtc;
-   int i;
 
cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
  (CIRRUSFB_CONN_LIMIT * sizeof(struct 
drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
drm_mode_crtc_set_gamma_size(_crtc->base, CIRRUS_LUT_SIZE);
 

[PATCH v2 10/14] drm: mgag200: remove dead code and pointless local lut storage

[Peter Rosin]

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.

Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ---
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  2 --
 drivers/gpu/drm/mgag200/mgag200_mode.c | 62 --
 3 files changed, 15 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h 
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec..04f1dfb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo)
 {
return container_of(bo, struct mgag200_bo, bo);
 }
-   /* mgag200_crtc.c */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
 
/* mgag200_mode.c */
 int mgag200_modeset_init(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c 
b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fa..5cf980a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev,
 }
 
 static const struct drm_fb_helper_funcs mga_fb_helper_funcs = {
-   .gamma_set = mga_crtc_fb_gamma_set,
-   .gamma_get = mga_crtc_fb_gamma_get,
.fb_probe = mgag200fb_create,
 };
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f4b5358..5e9cd4c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
 
 static void mga_crtc_load_lut(struct drm_crtc *crtc)
 {
-   struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct mga_device *mdev = dev->dev_private;
struct drm_framebuffer *fb = crtc->primary->fb;
+   u16 *r_ptr, *g_ptr, *b_ptr;
int i;
 
if (!crtc->enabled)
return;
 
+   r_ptr = crtc->gamma_store;
+   g_ptr = r_ptr + crtc->gamma_size;
+   b_ptr = g_ptr + crtc->gamma_size;
+
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
 
if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
if (i > (MGAG200_LUT_SIZE >> 1)) {
r = b = 0;
} else {
-   r = mga_crtc->lut_r[i << 1];
-   b = mga_crtc->lut_b[i << 1];
+   r = *r_ptr++ >> 8;
+   b = *b_ptr++ >> 8;
+   r_ptr++;
+   b_ptr++;
}
} else {
-   r = mga_crtc->lut_r[i];
-   b = mga_crtc->lut_b[i];
+   r = *r_ptr++ >> 8;
+   b = *b_ptr++ >> 8;
}
/* VGA registers */
WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
}
return;
}
for (i = 0; i < MGAG200_LUT_SIZE; i++) {
/* VGA registers */
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
}
 }
 
@@ -1399,14 +1405,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 
*red, u16 *green,
  u16 *blue, uint32_t size,
  struct drm_modeset_acquire_ctx *ctx)
 {
-   struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-   int i;
-
-   for (i = 0; i < size; i++) {
-   mga_crtc->lut_r[i] = red[i] >> 8;
-   mga_crtc->lut_g[i] = green[i] >> 8;
-   mga_crtc->lut_b[i] = blue[i] >> 8;
-   }
mga_crtc_load_lut(crtc);
 
return 0;
@@ -1455,14 +1453,12 @@ static const struct drm_crtc_helper_funcs 
mga_helper_funcs = {
.mode_set_base = 

[PATCH v2 00/14] improve the fb_setcmap helper

[Peter Rosin]

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.

I have obviously not tested all of this with more than a compile,
but patches 1 and 3 are enough to make the atmel-hlcdc driver
do what I need (when patched to support CLUT modes). The rest is
just lots of removals and cleanup made possible by the improved
core.

Please test, I would not be surprised if I have fouled up some
bit-manipulation somewhere in this mostly mechanical change...

Changes since v1:

- Rebased to next-20170621
- Split 1/11 into a preparatory patch, a cleanup patch and then
  the meat in 3/14.
- Handle pseudo-palette for FB_VISUAL_TRUECOLOR.
- Removed the empty .gamma_get/.gamma_set fb helpers from the
  armada driver that I had somehow managed to ignore but which
  0day found real quick.
- Be less judgemental on drivers only providing .gamma_get and
  .gamma_set, but no .load_lut. That's actually a valid thing
  to do if you only need pseudo-palette for FB_VISUAL_TRUECOLOR.
- Add a comment about colliding bitfields in the nouveau driver.
- Remove gamma_set/gamma_get declarations from the radeon driver
  (the definitions were removed in v1).

Cheers,
peda

Peter Rosin (14):
  drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
  drm/fb-helper: remove drm_fb_helper_save_lut_atomic
  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: armada: remove dead empty functions
  drm: ast: remove dead code and pointless local lut storage
  drm: cirrus: remove dead code and pointless local lut storage
  drm: 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/armada/armada_crtc.c|  10 --
 drivers/gpu/drm/armada/armada_crtc.h|   2 -
 drivers/gpu/drm/armada/armada_fbdev.c   |   2 -
 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 | 164 +---
 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  |  40 +++
 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/radeon/radeon_mode.h|   4 -
 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 ---
 40 files changed, 205 insertions(+), 658 deletions(-)

-- 
2.1.4

___
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

[Peter Rosin]

On 2017-06-22 08:36, Daniel Vetter wrote:
> 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.

I did the split in v2, I assume that's ok too. Better in case anyone ever
needs to run a bisect on this...

>>> 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 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 :-/

Ahh, ok, I see the problem.

> Using the dpms patch as template it shouldn't be too hard to address that
> for your patch here too.

Hmm, in that patch you handle the legacy case in a separate function, and
doing that for the lut case looks difficult when the atomic commit happens
inside the helper (typically drm_atomic_helper_legacy_gamma_set which
could perhaps be handled, but a real drag to handle for drivers that have
a custom crtc .gamma_set).

So, I'm aiming for the drop approach...

However, I don't have all of that series, and I suspect that is why I do
not have any fb_helper->lock.

I'll send my best guess as a follow-up to patch 3/14 in v2.

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

RE: [PATCH v2] 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 11:13 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Cui, Flora
> Subject: [PATCH v2] drm/amdgpu: export gpu always on cu bitmap


Tweak the subject and commit message a bit.  Something like:

drm/amdgpu: Fix the exported always on CU bitmap

Newer asics with 4 SEs are not able to fit the entire bitmask in the original 
field, use
an array instead.

With that,
Reviewed-by: Alex Deucher 

> 
> 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_PATCHLEVEL0
> 
>  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(_info.cu_ao_bitmap[0], 
> >gfx.cu_info.ao_cu_bitmap[0],
> +sizeof(adev->gfx.cu_info.ao_cu_bitmap));
>   memcpy(_info.cu_bitmap[0], 
> >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, 

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

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Wednesday, June 21, 2017 11:41 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH xf86-video-ati] Only call drmmode_scanout_free for non-
> GPU screens in LeaveVT
> 
> 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 

Reviewed-by: Alex Deucher 

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

Re: [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Philippe CORNU]

On 06/22/2017 08:06 AM, Peter Rosin wrote:
> The redundant fb helper .load_lut is no longer used, and can not
> work right without also providing the fb helpers .gamma_set and
> .gamma_get thus rendering the code in this driver suspect.
> 

Hi Peter,
STM32 chipsets supports 8-bit CLUT mode but this driver version does not 
support it "yet" (final patch has not been upstreamed because it was a 
too big fbdev patch for simply adding CLUT...).

Regarding your patch below, if it helps you to ease the drm framework 
update then I am agree to "acknowledge it" asap, else if you are not in 
a hurry, I would prefer a better and definitive patch handling 8-bit 
CLUT properly and I am ok to help or/and to do it : )

Extra questions:
- any plan to update modetest with the DRM_FORMAT_C8 support + gamma 
get/set?
- do you have a simple way to test clut with fbdev, last year we where 
using an old version of the SDL but I am still looking for a small piece 
of code to do it (else I will do it myself but C8 on fbdev is not really 
a priority ;-)

best regards,
Philippe

> Just remove the dead code.
> 
> Signed-off-by: Peter Rosin 
> ---
>   drivers/gpu/drm/stm/ltdc.c | 12 
>   drivers/gpu/drm/stm/ltdc.h |  1 -
>   2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 1b9483d..87829b9 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
>* DRM_CRTC
>*/
>   
> -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> -{
> - struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> - unsigned int i, lay;
> -
> - for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> - for (i = 0; i < 256; i++)
> - reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> -   ldev->clut[i]);
> -}
> -
>   static void ltdc_crtc_enable(struct drm_crtc *crtc)
>   {
>   struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
>   }
>   
>   static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> - .load_lut = ltdc_crtc_load_lut,
>   .enable = ltdc_crtc_enable,
>   .disable = ltdc_crtc_disable,
>   .mode_set_nofb = ltdc_crtc_mode_set_nofb,
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index d7a9c73..620ca55 100644
> --- a/drivers/gpu/drm/stm/ltdc.h
> +++ b/drivers/gpu/drm/stm/ltdc.h
> @@ -27,7 +27,6 @@ struct ltdc_device {
>   struct drm_panel *panel;
>   struct mutex err_lock;  /* protecting error_status */
>   struct ltdc_caps caps;
> - u32 clut[256];  /* color look up table */
>   u32 error_status;
>   u32 irq_status;
>   };
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu] Improve drmmode_fb_reference debugging code

[Michel Dänzer]

From: Michel Dänzer 

If a reference count is <= 0, call FatalError with the call location
(in case it doesn't get resolved in the backtrace printed by
FatalError).

Signed-off-by: Michel Dänzer 
---
 src/drmmode_display.h | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index b64a938cd..f351bb09c 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -133,32 +133,36 @@ enum drmmode_flip_sync {
 
 
 static inline void
-drmmode_fb_reference(int drm_fd, struct drmmode_fb **old, struct drmmode_fb 
*new)
+drmmode_fb_reference_loc(int drm_fd, struct drmmode_fb **old, struct 
drmmode_fb *new,
+const char *caller, unsigned line)
 {
if (new) {
if (new->refcnt <= 0) {
-   ErrorF("New FB's refcnt was %d in %s\n", new->refcnt,
-  __func__);
-   } else {
-   new->refcnt++;
+   FatalError("New FB's refcnt was %d at %s:%u",
+  new->refcnt, caller, line);
}
+
+   new->refcnt++;
}
 
if (*old) {
if ((*old)->refcnt <= 0) {
-   ErrorF("Old FB's refcnt was %d in %s\n",
-  (*old)->refcnt, __func__);
-   } else {
-   if (--(*old)->refcnt == 0) {
-   drmModeRmFB(drm_fd, (*old)->handle);
-   free(*old);
-   }
+   FatalError("Old FB's refcnt was %d at %s:%u",
+  (*old)->refcnt, caller, line);
+   }
+
+   if (--(*old)->refcnt == 0) {
+   drmModeRmFB(drm_fd, (*old)->handle);
+   free(*old);
}
}
 
*old = new;
 }
 
+#define drmmode_fb_reference(fd, old, new) \
+   drmmode_fb_reference_loc(fd, old, new, __func__, __LINE__)
+
 
 extern int drmmode_page_flip_target_absolute(AMDGPUEntPtr pAMDGPUEnt,
 drmmode_crtc_private_ptr 
drmmode_crtc,
-- 
2.11.0

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

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

[Christian König]

Am 22.06.2017 um 05:13 schrieb Flora Cui:

v2: keep cu_ao_mask for backward compatibility.

Change-Id: I056d8af23340d46e5140bd10cc38dfb887cc78ab
Signed-off-by: Flora Cui 


I can't judge the technical correctness, but at least it looks backward 
compatible now.


Patch is Acked-by: Christian König .

Regards,
Christian.


---
  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(_info.cu_ao_bitmap[0], 
>gfx.cu_info.ao_cu_bitmap[0],
+  sizeof(adev->gfx.cu_info.ao_cu_bitmap));
memcpy(_info.cu_bitmap[0], >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 |= 

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

[Christian König]

Am 22.06.2017 um 04:42 schrieb Alex Xie:

Signed-off-by: Alex Xie 


With the commit message fixed as Michel suggested patches #1 and #2 are 
Reviewed-by: Christian König  as well.


On patch #3 Marek needs to take a look, cause I don't know the logic 
behind that.


Regards,
Christian.


---
  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,
 >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,



___
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

[Boris Brezillon]

On Thu, 22 Jun 2017 08:37:55 +0200
Daniel Vetter  wrote:

> 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?

Already fixed in v2 ;-).
___
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

[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 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