[Nouveau] [PATCH] drm/nouveau: Do not leak client objects

[Ben Skeggs]

On 5 June 2015 at 21:35, Thierry Reding  wrote:
> On Thu, Oct 16, 2014 at 11:54:54AM +0200, Thierry Reding wrote:
>> From: Thierry Reding 
>>
>> The memory allocated for a nouveau_cli object in nouveau_cli_create() is
>> never freed. Free the memory in nouveau_cli_destroy() to plug this leak.
>>
>> kmemleak recorded this after running a couple of nouveau test programs.
>> Note that kmemleak points at drm_open_helper() because for some reason
>> it thinks that skipping the first two stack frames is a good idea.
>>
>> Signed-off-by: Thierry Reding 
>> ---
>>  drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
>> b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> index 57238076049f..6dc2c915ba6e 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
>> @@ -126,6 +126,7 @@ nouveau_cli_destroy(struct nouveau_cli *cli)
>>   nouveau_vm_ref(NULL, &nvkm_client(&cli->base)->vm, NULL);
>>   nvif_client_fini(&cli->base);
>>   usif_client_fini(cli);
>> + kfree(cli);
>>  }
>>
>>  static void
>
> Ben, any chance you could pick this up?
Got it :)  Thanks for the ping!

Ben.

>
> Thierry
>
> ___
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau
>

[PATCH] drm/panel: Add display timing for Okaya RS800480T-7X0GP

[Gary Bisson]

Thierry, All,

On Fri, Jun 5, 2015 at 2:28 PM, Thierry Reding  
wrote:
> On Thu, May 28, 2015 at 05:37:46PM +0200, Gary Bisson wrote:
>> Add support for the Okaya RS800480T-7X0GP to the DRM simple panel
>> driver.
>>
>> The RS800480T-7X0GP is a WVGA (800x480) panel with an 18-bit parallel
>> LCD interface. It supports pixel clocks in the range of 30-40 MHz.
>>
>> This panel details can be found at:
>> http://boundarydevices.com/product/7-800x480-display/
>>
>> Signed-off-by: Gary Bisson 
>> ---
>> Hi all,
>>
>> This patch is the follow-up of a request from Philipp to add the Okaya 
>> display
>> to the simple panel driver.
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/346657.html
>>
>> Regards,
>> Gary
>> ---
>>  .../bindings/panel/okaya,rs800480t_7x0gp.txt   |  7 ++
>>  .../devicetree/bindings/vendor-prefixes.txt|  1 +
>>  drivers/gpu/drm/panel/panel-simple.c   | 27 
>> ++
>>  3 files changed, 35 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/panel/okaya,rs800480t_7x0gp.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/panel/okaya,rs800480t_7x0gp.txt 
>> b/Documentation/devicetree/bindings/panel/okaya,rs800480t_7x0gp.txt
>> new file mode 100644
>> index 000..f7c729d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/okaya,rs800480t_7x0gp.txt
>> @@ -0,0 +1,7 @@
>> +OKAYA Electric America, Inc. RS800480T-7X0GP 7" WVGA LCD panel
>> +
>> +Required properties:
>> +- compatible: should be "okaya,rs800480t_7x0gp"
>> +
>> +This binding is compatible with the simple-panel binding, which is specified
>> +in simple-panel.txt in this directory.
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
>> b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 4f35a00..06ce91c 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -145,6 +145,7 @@ nintendo  Nintendo
>>  nokiaNokia
>>  nvidia   NVIDIA
>>  nxp  NXP Semiconductors
>> +okayaOKAYA Electric America, Inc.
>>  onnn ON Semiconductor Corp.
>>  opencoresOpenCores.org
>>  ortustechOrtus Technology Co., Ltd.
>
> Can you split this change into a separate patch? It needs an Acked-by
> from one of the device tree binding maintainers, so make sure to Cc them
> when you repost. scripts/get_maintainer.pl will list them for you.

Sure, will do.

>> +static const struct display_timing okaya_rs800480t_7x0gp_timing = {
>> + .pixelclock = { 3000, 3000, 4000 },
>> + .hactive = { 800, 800, 800 },
>> + .hfront_porch = { 40, 40, 40 },
>> + .hback_porch = { 40, 40, 40 },
>> + .hsync_len = { 1, 48, 48 },
>> + .vactive = { 480, 480, 480 },
>> + .vfront_porch = { 13, 13, 13 },
>> + .vback_porch = { 29, 29, 29 },
>> + .vsync_len = { 3, 3, 3 },
>
> It strikes me as odd that the porches and VSYNC width should be fixed
> for the panel. Is this really the case?

Well this is the values provided by Okaya. Here is a link to the
datasheet, details are located in page 8:
http://boundarydevices.com/datasheets/RS800480T-7X0GP-A.pdf

Thanks for the feedback,
Gary

[PATCH 49/88] drm/amdgpu: remove AMDGPU_GEM_CREATE_CPU_GTT_UC

[Oded Gabbay]

On Sat, Jun 6, 2015 at 10:09 PM, Christian König
 wrote:
> On 06.06.2015 19:08, Oded Gabbay wrote:
>
> Hi Alex,
> I think you have a mistake in this patch.
> You renamed AMDGPU_GEM_CREATE_CPU_GTT_WC to AMDGPU_GEM_CREATE_CPU_GTT_USWC,
> however, AMDGPU_GEM_CREATE_CPU_GTT_WC was defined as (1 << 3) and
> AMDGPU_GEM_CREATE_CPU_GTT_USWC is defined as (1 << 2)
>
>
> That patch merged AMDGPU_GEM_CREATE_CPU_GTT_WC and
> AMDGPU_GEM_CREATE_CPU_GTT_UC to AMDGPU_GEM_CREATE_CPU_GTT_USWC.
>
> So using 1 << 2 for the new value is perfectly fine.
>
> Christian.
>
Yeah, I see it now.
False alarm.

Oded
>
>
> Oded
>
> On Wed, May 27, 2015 at 6:22 AM Alex Deucher  wrote:
>>
>> From: Jammy Zhou 
>>
>> This flag isn't used by user mode drivers, remove it to avoid
>> confusion. And rename GTT_WC to GTT_USWC to make it clear.
>>
>> Signed-off-by: Jammy Zhou 
>> Reviewed-by: Alex Deucher 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++
>>  include/uapi/drm/amdgpu_drm.h  |  7 ++-
>>  2 files changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index f5e17f9..992b7f5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -132,10 +132,7 @@ void amdgpu_ttm_placement_from_domain(struct
>> amdgpu_bo *rbo, u32 domain)
>> }
>>
>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
>> -   rbo->placements[c].fpfn = 0;
>> -   rbo->placements[c++].flags = TTM_PL_FLAG_UNCACHED
>> | TTM_PL_FLAG_TT;
>> -   } else if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_WC) {
>> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
>> rbo->placements[c].fpfn = 0;
>> rbo->placements[c++].flags = TTM_PL_FLAG_WC |
>> TTM_PL_FLAG_TT |
>>
>> TTM_PL_FLAG_UNCACHED;
>> @@ -146,10 +143,7 @@ void amdgpu_ttm_placement_from_domain(struct
>> amdgpu_bo *rbo, u32 domain)
>> }
>>
>> if (domain & AMDGPU_GEM_DOMAIN_CPU) {
>> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
>> -   rbo->placements[c].fpfn = 0;
>> -   rbo->placements[c++].flags =  TTM_PL_FLAG_UNCACHED
>> | TTM_PL_FLAG_SYSTEM;
>> -   } else if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_WC) {
>> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
>> rbo->placements[c].fpfn = 0;
>> rbo->placements[c++].flags = TTM_PL_FLAG_WC |
>> TTM_PL_FLAG_SYSTEM |
>>
>> TTM_PL_FLAG_UNCACHED;
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 9e771fb..77bc574 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -73,15 +73,12 @@
>>  #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED  (1 << 0)
>>  /* Flag that CPU access will not work, this VRAM domain is invisible */
>>  #define AMDGPU_GEM_CREATE_NO_CPU_ACCESS(1 << 1)
>> -/* Flag that un-cached attributes should be used for GTT */
>> -#define AMDGPU_GEM_CREATE_CPU_GTT_UC   (1 << 2)
>>  /* Flag that USWC attributes should be used for GTT */
>> -#define AMDGPU_GEM_CREATE_CPU_GTT_WC   (1 << 3)
>> +#define AMDGPU_GEM_CREATE_CPU_GTT_USWC (1 << 2)
>>
>>  /* Flag mask for GTT domain_flags */
>>  #define AMDGPU_GEM_CREATE_CPU_GTT_MASK \
>> -   (AMDGPU_GEM_CREATE_CPU_GTT_WC | \
>> -AMDGPU_GEM_CREATE_CPU_GTT_UC | \
>> +   (AMDGPU_GEM_CREATE_CPU_GTT_USWC | \
>>  AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | \
>>  AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>>
>> --
>> 1.8.3.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

[PATCH] drm/amdkfd: remove not used defines from cik_regs.h

[Oded Gabbay]

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdkfd/cik_regs.h | 177 +++---
 1 file changed, 13 insertions(+), 164 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/cik_regs.h 
b/drivers/gpu/drm/amd/amdkfd/cik_regs.h
index 01ff332..183be5b 100644
--- a/drivers/gpu/drm/amd/amdkfd/cik_regs.h
+++ b/drivers/gpu/drm/amd/amdkfd/cik_regs.h
@@ -23,33 +23,11 @@
 #ifndef CIK_REGS_H
 #define CIK_REGS_H

-#define IH_VMID_0_LUT  0x3D40u
-
-#define BIF_DOORBELL_CNTL  0x530Cu
-
-#defineSRBM_GFX_CNTL   0xE44
-#definePIPEID(x)   ((x) << 0)
-#defineMEID(x) ((x) << 2)
-#defineVMID(x) ((x) << 4)
-#defineQUEUEID(x)  ((x) << 8)
-
-#defineSQ_CONFIG   0x8C00
-
-#defineSH_MEM_BASES0x8C28
 /* if PTR32, these are the bases for scratch and lds */
 #definePRIVATE_BASE(x) ((x) << 0) /* 
scratch */
 #defineSHARED_BASE(x)  ((x) << 16) /* 
LDS */
-#defineSH_MEM_APE1_BASE0x8C2C
-/* if PTR32, this is the base location of GPUVM */
-#defineSH_MEM_APE1_LIMIT   0x8C30
-/* if PTR32, this is the upper limit of GPUVM */
-#defineSH_MEM_CONFIG   0x8C34
 #definePTR32   (1 << 0)
-#define PRIVATE_ATC(1 << 1)
 #defineALIGNMENT_MODE(x)   ((x) << 2)
-#defineSH_MEM_ALIGNMENT_MODE_DWORD 0
-#defineSH_MEM_ALIGNMENT_MODE_DWORD_STRICT  1
-#defineSH_MEM_ALIGNMENT_MODE_STRICT2
 #defineSH_MEM_ALIGNMENT_MODE_UNALIGNED 3
 #defineDEFAULT_MTYPE(x)((x) << 4)
 #defineAPE1_MTYPE(x)   ((x) << 7)
@@ -58,137 +36,34 @@
 #defineMTYPE_CACHED0
 #defineMTYPE_NONCACHED 3

-
-#define SH_STATIC_MEM_CONFIG   0x9604u
-
-#defineTC_CFG_L1_LOAD_POLICY0  0xAC68
-#defineTC_CFG_L1_LOAD_POLICY1  0xAC6C
-#defineTC_CFG_L1_STORE_POLICY  0xAC70
-#defineTC_CFG_L2_LOAD_POLICY0  0xAC74
-#defineTC_CFG_L2_LOAD_POLICY1  0xAC78
-#defineTC_CFG_L2_STORE_POLICY0 0xAC7C
-#defineTC_CFG_L2_STORE_POLICY1 0xAC80
-#defineTC_CFG_L2_ATOMIC_POLICY 0xAC84
-#defineTC_CFG_L1_VOLATILE  0xAC88
-#defineTC_CFG_L2_VOLATILE  0xAC8C
-
-#define CP_PQ_WPTR_POLL_CNTL   0xC20C
-#defineWPTR_POLL_EN(1 << 31)
-
-#define CPC_INT_CNTL   0xC2D0
-#define CP_ME1_PIPE0_INT_CNTL  0xC214
-#define CP_ME1_PIPE1_INT_CNTL  0xC218
-#define CP_ME1_PIPE2_INT_CNTL  0xC21C
-#define CP_ME1_PIPE3_INT_CNTL  0xC220
-#define CP_ME2_PIPE0_INT_CNTL  0xC224
-#define CP_ME2_PIPE1_INT_CNTL  0xC228
-#define CP_ME2_PIPE2_INT_CNTL  0xC22C
-#define CP_ME2_PIPE3_INT_CNTL  0xC230
-#define DEQUEUE_REQUEST_INT_ENABLE (1 << 13)
-#define WRM_POLL_TIMEOUT_INT_ENABLE(1 << 17)
-#define PRIV_REG_INT_ENABLE(1 << 23)
-#define TIME_STAMP_INT_ENABLE  (1 << 26)
-#define GENERIC2_INT_ENABLE(1 << 29)
-#define GENERIC1_INT_ENABLE(1 << 30)
-#define GENERIC0_INT_ENABLE(1 << 31)
-#define CP_ME1_PIPE0_INT_STATUS0xC214
-#define CP_ME1_PIPE1_INT_STATUS0xC218
-#define CP_ME1_PIPE2_INT_STATUS0xC21C
-#define CP_ME1_PIPE3_INT_STATUS0xC220
-#define CP_ME2_PIPE0_INT_STATUS0xC224
-#define CP_ME2_PIPE1_INT_STATUS0xC228
-#define CP_ME2_PIPE2_INT_STATUS0xC22C
-#define CP_ME2_PIPE3_INT_STATUS0xC230
-#define DEQUEUE_REQUEST_INT_STATUS (1 << 13)
-#d

[PATCH] drm/amdkfd: Add missing properties to CZ device info

[Oded Gabbay]

This patch adds two missing properties initializations to the device
info structure of CZ.

As we don't have CZ support yet, it isn't critical, but its important to
fix this now instead of forgetting about it later.

Signed-off-by: Oded Gabbay 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 1d1e2e9..75312c8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -44,7 +44,10 @@ static const struct kfd_device_info kaveri_device_info = {
 static const struct kfd_device_info carrizo_device_info = {
.asic_family = CHIP_CARRIZO,
.max_pasid_bits = 16,
+   /* max num of queues for CZ.TODO should be a dynamic value */
+   .max_no_of_hqd  = 24,
.ih_ring_entry_size = 4 * sizeof(uint32_t),
+   .event_interrupt_class = &event_interrupt_class_cik,
.num_of_watch_points = 4,
.mqd_size_aligned = MQD_SIZE_ALIGNED
 };
-- 
2.4.2

[PATCH] drm/amdkfd: do reset wavefronts per process per device

[Oded Gabbay]

From: Ben Goz 

This commit moves the reset wavefront flag to per process per device
data structure, so we can support multiple devices.

Signed-off-by: Ben Goz 
Signed-off-by: Oded Gabbay 
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   |  7 ---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h   | 10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c| 21 -
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 547b0a5..4bb7f42 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -946,7 +946,7 @@ static int destroy_queues_cpsch(struct device_queue_manager 
*dqm,
 {
int retval;
enum kfd_preempt_type_filter preempt_type;
-   struct kfd_process *p;
+   struct kfd_process_device *pdd;

BUG_ON(!dqm);

@@ -981,8 +981,9 @@ static int destroy_queues_cpsch(struct device_queue_manager 
*dqm,
retval = amdkfd_fence_wait_timeout(dqm->fence_addr, KFD_FENCE_COMPLETED,
QUEUE_PREEMPT_DEFAULT_TIMEOUT_MS);
if (retval != 0) {
-   p = kfd_get_process(current);
-   p->reset_wavefronts = true;
+   pdd = kfd_get_process_device_data(dqm->dev,
+   kfd_get_process(current));
+   pdd->reset_wavefronts = true;
goto out;
}
pm_release_ib(&dqm->packets);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index cb79046..d0d5f4b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -463,6 +463,11 @@ struct kfd_process_device {

/* Is this process/pasid bound to this device? (amd_iommu_bind_pasid) */
bool bound;
+
+   /* This flag tells if we should reset all
+* wavefronts on process termination
+*/
+   bool reset_wavefronts;
 };

 #define qpd_to_pdd(x) container_of(x, struct kfd_process_device, qpd)
@@ -519,11 +524,6 @@ struct kfd_process {
event_pages */
u32 next_nonsignal_event_id;
size_t signal_event_count;
-   /*
-* This flag tells if we should reset all wavefronts on
-* process termination
-*/
-   bool reset_wavefronts;
 };

 /**
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 56b904f..8a1f999 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -173,7 +173,7 @@ static void kfd_process_wq_release(struct work_struct *work)
pr_debug("Releasing pdd (topology id %d) for process (pasid %d) 
in workqueue\n",
pdd->dev->id, p->pasid);

-   if (p->reset_wavefronts)
+   if (pdd->reset_wavefronts)
dbgdev_wave_reset_wavefronts(pdd->dev, p);

amd_iommu_unbind_pasid(pdd->dev->pdev, p->pasid);
@@ -222,6 +222,7 @@ static void kfd_process_notifier_release(struct 
mmu_notifier *mn,
struct mm_struct *mm)
 {
struct kfd_process *p;
+   struct kfd_process_device *pdd = NULL;

/*
 * The kfd_process structure can not be free because the
@@ -240,6 +241,15 @@ static void kfd_process_notifier_release(struct 
mmu_notifier *mn,
/* In case our notifier is called before IOMMU notifier */
pqm_uninit(&p->pqm);

+   /* Iterate over all process device data structure and check
+* if we should reset all wavefronts */
+   list_for_each_entry(pdd, &p->per_device_data, per_device_list)
+   if (pdd->reset_wavefronts) {
+   pr_warn("amdkfd: Resetting all wave fronts\n");
+   dbgdev_wave_reset_wavefronts(pdd->dev, p);
+   pdd->reset_wavefronts = false;
+   }
+
mutex_unlock(&p->mutex);

/*
@@ -305,8 +315,6 @@ static struct kfd_process *create_process(const struct 
task_struct *thread)
if (kfd_init_apertures(process) != 0)
goto err_init_apretures;

-   process->reset_wavefronts = false;
-
return process;

 err_init_apretures:
@@ -348,6 +356,7 @@ struct kfd_process_device 
*kfd_create_process_device_data(struct kfd_dev *dev,
INIT_LIST_HEAD(&pdd->qpd.queues_list);
INIT_LIST_HEAD(&pdd->qpd.priv_queue_list);
pdd->qpd.dqm = dev->dqm;
+   pdd->reset_wavefronts = false;
list_add(&pdd->per_device_list, &p->per_device_data);
}

@@ -409,10 +418,12 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev, 
unsigned int pasid)
kfd_dbgmgr_destroy(dev->dbgmgr);

pqm_uninit

[PATCH 49/88] drm/amdgpu: remove AMDGPU_GEM_CREATE_CPU_GTT_UC

[Christian König]

On 06.06.2015 19:08, Oded Gabbay wrote:
> Hi Alex,
> I think you have a mistake in this patch.
> You renamed AMDGPU_GEM_CREATE_CPU_GTT_WC to 
> AMDGPU_GEM_CREATE_CPU_GTT_USWC, however, AMDGPU_GEM_CREATE_CPU_GTT_WC 
> was defined as (1 << 3) and AMDGPU_GEM_CREATE_CPU_GTT_USWC is defined 
> as (1 << 2)

That patch merged AMDGPU_GEM_CREATE_CPU_GTT_WC and 
AMDGPU_GEM_CREATE_CPU_GTT_UC to AMDGPU_GEM_CREATE_CPU_GTT_USWC.

So using 1 << 2 for the new value is perfectly fine.

Christian.

>
> Oded
>
> On Wed, May 27, 2015 at 6:22 AM Alex Deucher  <mailto:alexdeucher at gmail.com>> wrote:
>
> From: Jammy Zhou mailto:Jammy.Zhou at amd.com>>
>
> This flag isn't used by user mode drivers, remove it to avoid
> confusion. And rename GTT_WC to GTT_USWC to make it clear.
>
> Signed-off-by: Jammy Zhou  <mailto:Jammy.Zhou at amd.com>>
> Reviewed-by: Alex Deucher  <mailto:alexander.deucher at amd.com>>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++
>  include/uapi/drm/amdgpu_drm.h  |  7 ++-
>  2 files changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index f5e17f9..992b7f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -132,10 +132,7 @@ void amdgpu_ttm_placement_from_domain(struct
> amdgpu_bo *rbo, u32 domain)
> }
>
> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
> -   rbo->placements[c].fpfn = 0;
> -   rbo->placements[c++].flags =
> TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_TT;
> -   } else if (rbo->flags &
> AMDGPU_GEM_CREATE_CPU_GTT_WC) {
> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
> rbo->placements[c].fpfn = 0;
> rbo->placements[c++].flags =
> TTM_PL_FLAG_WC | TTM_PL_FLAG_TT |
>TTM_PL_FLAG_UNCACHED;
> @@ -146,10 +143,7 @@ void amdgpu_ttm_placement_from_domain(struct
> amdgpu_bo *rbo, u32 domain)
> }
>
> if (domain & AMDGPU_GEM_DOMAIN_CPU) {
> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
> -   rbo->placements[c].fpfn = 0;
> -   rbo->placements[c++].flags =
> TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_SYSTEM;
> -   } else if (rbo->flags &
> AMDGPU_GEM_CREATE_CPU_GTT_WC) {
> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
> rbo->placements[c].fpfn = 0;
> rbo->placements[c++].flags =
> TTM_PL_FLAG_WC | TTM_PL_FLAG_SYSTEM |
>TTM_PL_FLAG_UNCACHED;
> diff --git a/include/uapi/drm/amdgpu_drm.h
> b/include/uapi/drm/amdgpu_drm.h
> index 9e771fb..77bc574 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -73,15 +73,12 @@
>  #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED  (1 << 0)
>  /* Flag that CPU access will not work, this VRAM domain is
> invisible */
>  #define AMDGPU_GEM_CREATE_NO_CPU_ACCESS(1 << 1)
> -/* Flag that un-cached attributes should be used for GTT */
> -#define AMDGPU_GEM_CREATE_CPU_GTT_UC   (1 << 2)
>  /* Flag that USWC attributes should be used for GTT */
> -#define AMDGPU_GEM_CREATE_CPU_GTT_WC   (1 << 3)
> +#define AMDGPU_GEM_CREATE_CPU_GTT_USWC (1 << 2)
>
>  /* Flag mask for GTT domain_flags */
>  #define AMDGPU_GEM_CREATE_CPU_GTT_MASK \
> -   (AMDGPU_GEM_CREATE_CPU_GTT_WC | \
> -AMDGPU_GEM_CREATE_CPU_GTT_UC | \
> +   (AMDGPU_GEM_CREATE_CPU_GTT_USWC | \
>  AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | \
>  AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> <mailto:dri-devel at lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/87f3da71/attachment.html>

[PATCH] drm/amdkfd: avoid CONFIG_ prefix for non-Kconfig symbols

[Oded Gabbay]

On Fri, Jun 5, 2015 at 11:27 AM, Valentin Rothberg
 wrote:
> The CONFIG_ prefix is reserved for Kconfig options in Make and CPP
> syntax.  Various static analysis tools rely on this naming convention
> and check if CONFIG_ prefixed symbols are defined Kconfig.  Hence add
> yet another prefix AMD_ to CONFIG_REG_{BASE,END,SISE} to apply to this
> convention and make static analysis tools happy.
>
> Signed-off-by: Valentin Rothberg 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 10 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> index 96153f28d73f..c34c393e9aea 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
> @@ -445,7 +445,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev 
> *dbgdev,
> aw_reg_add_dword /= sizeof(uint32_t);
>
> packets_vec[0].bitfields2.reg_offset =
> -   aw_reg_add_dword - CONFIG_REG_BASE;
> +   aw_reg_add_dword - 
> AMD_CONFIG_REG_BASE;
>
> packets_vec[0].reg_data[0] = cntl.u32All;
>
> @@ -458,7 +458,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev 
> *dbgdev,
> aw_reg_add_dword /= sizeof(uint32_t);
>
> packets_vec[1].bitfields2.reg_offset =
> -   aw_reg_add_dword - CONFIG_REG_BASE;
> +   aw_reg_add_dword - 
> AMD_CONFIG_REG_BASE;
> packets_vec[1].reg_data[0] = addrHi.u32All;
>
> aw_reg_add_dword =
> @@ -470,7 +470,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev 
> *dbgdev,
> aw_reg_add_dword /= sizeof(uint32_t);
>
> packets_vec[2].bitfields2.reg_offset =
> -   aw_reg_add_dword - CONFIG_REG_BASE;
> +   aw_reg_add_dword - AMD_CONFIG_REG_BASE;
> packets_vec[2].reg_data[0] = addrLo.u32All;
>
> /* enable watch flag if address is not zero*/
> @@ -488,7 +488,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev 
> *dbgdev,
> aw_reg_add_dword /= sizeof(uint32_t);
>
> packets_vec[3].bitfields2.reg_offset =
> -   aw_reg_add_dword - CONFIG_REG_BASE;
> +   aw_reg_add_dword - 
> AMD_CONFIG_REG_BASE;
> packets_vec[3].reg_data[0] = cntl.u32All;
>
> status = dbgdev_diq_submit_ib(
> @@ -690,7 +690,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev 
> *dbgdev,
> packets_vec[1].header.opcode = IT_SET_CONFIG_REG;
> packets_vec[1].header.type = PM4_TYPE_3;
> packets_vec[1].bitfields2.reg_offset = SQ_CMD / (sizeof(uint32_t)) -
> -   CONFIG_REG_BASE;
> +   AMD_CONFIG_REG_BASE;
>
> packets_vec[1].bitfields2.vmid_shift = SQ_CMD_VMID_OFFSET;
> packets_vec[1].bitfields2.insert_vmid = 1;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h
> index 4b0dd5aa5306..03424c20920c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.h
> @@ -48,9 +48,9 @@ enum {
>
>  /* CONFIG reg space definition */
>  enum {
> -   CONFIG_REG_BASE = 0x2000,   /* in dwords */
> -   CONFIG_REG_END = 0x2B00,
> -   CONFIG_REG_SIZE = CONFIG_REG_END - CONFIG_REG_BASE
> +   AMD_CONFIG_REG_BASE = 0x2000,   /* in dwords */
> +   AMD_CONFIG_REG_END = 0x2B00,
> +   AMD_CONFIG_REG_SIZE = AMD_CONFIG_REG_END - AMD_CONFIG_REG_BASE
>  };
>
>  /* SH reg space definition */
> --
> 2.4.2
>

Thanks!
Applied to my -next-fixes tree

Oded

drm/i915: WARN_ON(dev_priv->mm.busy)

[Jeremiah Mahler]

all,

On all my machines with Intel graphics I get the following warning
in the logs when the machine is suspended.  Apparently some part of
the graphics system is busy when it should be idle. This is present
on the latest linux-next 20150604.

  ...
  [   33.141747] Suspending console(s) (use no_console_suspend to debug)
  [   33.142146] wlan0: deauthenticating from 00:1a:70:5a:6e:0b by local
  choice (Reason: 3=DEAUTH_LEAVING)
  [   33.147395] queueing ieee80211 work while going to suspend
  [   33.151597] cfg80211: Calling CRDA to update world regulatory domain
  [   33.190430] sd 0:0:0:0: [sda] Synchronizing SCSI cache
  [   33.190523] sd 0:0:0:0: [sda] Stopping disk
  [   33.275743] [ cut here ]
  [   33.275764] WARNING: CPU: 0 PID: 1617 at
  drivers/gpu/drm/i915/i915_gem.c:4808 i915_gem_suspend+0xe4/0xf0 [i915]()
  [   33.275766] WARN_ON(dev_priv->mm.busy)
  [   33.275811] Modules linked in: binfmt_misc snd_hda_codec_hdmi
  hid_generic isl29018(C) industrialio regmap_i2c cyapatp crc_itu_t usbhid
  hid arc4 x86_pkg_temp_thermal intel_powerclamp intel_rapl iosf_mbi
  coretemp ath9k tpm_infineon kvm_intel kvm ath9k_common ath9k_hw
  crct10dif_pclmul crc32_pclmul crc32c_intel chromeos_laptop ath mac80211
  ghash_clmulni_intel cryptd i915 cfg80211 pcspkr serio_raw sg ath3k btusb
  btrtl lpc_ich snd_hda_codec_realtek shpchp i2c_i801 mfd_core
  snd_hda_codec_generic btbcm btintel bluetooth snd_hda_intel battery
  snd_hda_codec ac i2c_algo_bit drm_kms_helper tpm_tis snd_hwdep tpm
  snd_hda_core drm snd_pcm video rfkill processor button snd_timer snd
  soundcore i2c_designware_pci i2c_designware_core evdev uvcvideo
  videobuf2_vmalloc videobuf2_memops videobuf2_core v4l2_common videodev
  [   33.275825]  media i2c_core fuse autofs4 ext4 crc16 mbcache jbd2
  sd_mod fan xhci_pci sdhci_acpi sdhci xhci_hcd mmc_core thermal
  thermal_sys usbcore ahci libahci usb_common libata scsi_mod
  [   33.275828] CPU: 0 PID: 1617 Comm: kworker/u4:4 Tainted: G C
  4.1.0-rc6-next-20150604+ #207
  [   33.275829] Hardware name: Acer Peppy, BIOS  04/30/2014
  [   33.275834] Workqueue: events_unbound async_run_entry_fn
  [   33.275838]   a05b7908 8152ca4d
  880035effc58
  [   33.275840]  8106bce1 880073587f20 
  88007358
  [   33.275842]  88003534f860 88007358 8106bd5a
  a05c74c1
  [   33.275843] Call Trace:
  [   33.275849]  [] ? dump_stack+0x40/0x50
  [   33.275853]  [] ? warn_slowpath_common+0x81/0xb0
  [   33.275855]  [] ? warn_slowpath_fmt+0x4a/0x50
  [   33.275865]  [] ? i915_gem_suspend+0xe4/0xf0 [i915]
  [   33.275872]  [] ? i915_drm_suspend+0x61/0x1b0
  [i915]
  [   33.275876]  [] ? pci_pm_suspend+0x71/0x140
  [   33.275878]  [] ? pci_pm_freeze+0xd0/0xd0
  [   33.275881]  [] ? dpm_run_callback+0x39/0xd0
  [   33.275883]  [] ? __device_suspend+0xe4/0x300
  [   33.275884]  [] ? async_suspend+0x1e/0x90
  [   33.275887]  [] ? async_run_entry_fn+0x43/0x150
  [   33.275890]  [] ? process_one_work+0x148/0x3b0
  [   33.275892]  [] ? worker_thread+0x4a/0x440
  [   33.275895]  [] ? rescuer_thread+0x2e0/0x2e0
  [   33.275898]  [] ? kthread+0xc1/0xe0
  [   33.275901]  [] ?
  kthread_create_on_node+0x190/0x190
  [   33.275904]  [] ? ret_from_fork+0x3f/0x70
  [   33.275907]  [] ?
  kthread_create_on_node+0x190/0x190
  [   33.275908] ---[ end trace e1c3eb5e163b3520 ]---
  [   33.560558] PM: suspend of devices complete after 423.034 msecs
  [   33.577985] PM: late suspend of devices complete after 17.589 msecs
  [   33.579036] xhci_hcd :00:14.0: System wakeup enabled by ACPI
  [   33.594059] PM: noirq suspend of devices complete after 16.226 msecs
  [   33.594498] ACPI: Preparing to enter system sleep state S3
  [   33.595066] ACPI : EC: EC stopped
  ...

-- 
- Jeremiah Mahler

[PATCH v2 07/10] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Sharma, Shashank]

Regards
Shashank

On 6/6/2015 11:03 AM, Jindal, Sonika wrote:
>
>
> On 6/4/2015 7:12 PM, Kausal Malladi wrote:
>> From: Kausal Malladi 
>>
>> This patch does the following:
>> 1. Adds the core function to program Gamma correction values for CHV/BSW
>> platform
>> 2. Adds Gamma correction macros/defines
>> 3. Adds drm_mode_crtc_update_color_property function, which replaces the
>> old blob for the property with the new one
>> 4. Adds a pointer to hold blob for Gamma property in drm_crtc
>>
>> v2: Addressed all review comments from Sonika and Daniel Stone.
> Instead you can mention the changes briefly.
>
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   drivers/gpu/drm/i915/intel_atomic.c|   6 ++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 161
>> +
>>   drivers/gpu/drm/i915/intel_color_manager.h |  62 +++
>>   3 files changed, 229 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index b5c78d8..4726847 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -428,6 +428,12 @@ intel_crtc_atomic_set_property(struct drm_crtc
>> *crtc,
>>  struct drm_property *property,
>>  uint64_t val)
>>   {
>> +struct drm_device *dev = crtc->dev;
>> +struct drm_mode_config *config = &dev->mode_config;
>> +
>> +if (property == config->gamma_property)
>> +return intel_color_manager_set_gamma(dev, &crtc->base, val);
>> +
>>   DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>>   return -EINVAL;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 8d4ee8f..421c267 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,167 @@
>>
>>   #include "intel_color_manager.h"
>>
>> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
>> +struct drm_crtc *crtc)
>> +{
>> +struct drm_gamma *gamma_data;
>> +struct drm_i915_private *dev_priv = dev->dev_private;
>> +struct drm_property_blob *blob;
>> +struct drm_mode_config *config = &dev->mode_config;
>> +u32 cgm_control_reg = 0;
>> +u32 cgm_gamma_reg = 0;
>> +u32 reg, val;
>> +enum pipe pipe;
>> +u16 red, green, blue;
>> +struct rgb_pixel correct_rgb;
>> +u32 count = 0;
>> +struct rgb_pixel *correction_values = NULL;
>> +u32 num_samples;
>> +u32 word;
>> +u32 palette;
>> +int ret = 0, length;
>> +
>> +blob = drm_property_lookup_blob(dev, blob_id);
>> +if (!blob) {
>> +DRM_ERROR("Invalid Blob ID\n");
>> +return -EINVAL;
>> +}
>> +
>> +gamma_data = (struct drm_gamma *)blob->data;
>> +pipe = to_intel_crtc(crtc)->pipe;
>> +num_samples = gamma_data->num_samples;
>> +length = num_samples * sizeof(struct rgb_pixel);
>> +
>> +if (gamma_data->gamma_precision == I915_GAMMA_PRECISION_UNKNOWN) {
> Switch/case instead?
Yep, got it.
>
> Also, is UNKNOWN for disabling? Why not rename it to DISABLE then?
Actually unknown is valid in case of get_property() when we want to 
query about the capabilities, just want to reuse the same, to avoid need 
for another one. Else we have to handle one extra case in each get_prop 
(disable) and set_prop(unknown)
>> +
>> +/* Disable Gamma functionality on Pipe - CGM Block */
>> +cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
>> +cgm_control_reg &= ~CGM_GAMMA_EN;
>> +I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
>> +
>> +DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
>> +pipe_name(pipe));
>> +ret = 0;
>> +} else if (gamma_data->gamma_precision ==
>> I915_GAMMA_PRECISION_LEGACY) {
>> +
>> +if (num_samples != CHV_8BIT_GAMMA_MAX_VALS) {
>> +DRM_ERROR("Incorrect number of samples received\n");
>> +return -EINVAL;
>> +}
>> +
>> +/* First, disable CGM Gamma, if already set */
>> +cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
>> +cgm_control_reg &= ~CGM_GAMMA_EN;
>> +I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
>> +
>> +/* Enable (Legacy) Gamma on Pipe */
>> +palette = _PIPE_GAMMA_BASE(pipe);
>> +
>> +count = 0;
>> +correction_values = (struct rgb_pixel *)&gamma_data->values;
>> +while (count < num_samples) {
>> +correct_rgb = correction_values[count];
>> +blue = correction_values[count].blue;
>> +green = correction_values[count].green;
>> +red = correction_values[count].red;
>> +
>> +blue = blue >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +green = green >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +red = red >> CHV_8BIT_GAMMA_MSB_SHIFT;
>> +
>> +/* Red (23:16),

[PATCH v2 06/10] drm: Avoid atomic commit path for CRTC property (Gamma)

[Sharma, Shashank]

Regards
Shashank

On 6/6/2015 6:31 AM, Matt Roper wrote:
> On Thu, Jun 04, 2015 at 07:12:37PM +0530, Kausal Malladi wrote:
>> From: Kausal Malladi 
>>
>> The atomic CRTC set infrastructure is not available yet. So, the atomic
>> check path throws error for any non-plane property.
>>
>> This patch adds a diversion to avoid commit path for DRM atomic set Gamma
>> property, until atomic CRTC infrastructure is ready.
>
> This doesn't look right, but I don't quite understand what you're trying
> to do from the commit message.
>
> This function is what will implement legacy set_property ioctl's of a
> CRTC on an atomic-based driver.  The idea is that when the ioctl is
> issued, we should get (i.e., make a duplicate of) the current CRTC
> state, change some of the values in that state (which is what the
> .atomic_set_property function takes care of), and then submit that
> modified state to the driver for checking and hw-programming.
>
> Okay, I just took a quick peek ahead in your patch set and I think I
> understand the confusion now...it looks like you're trying to actually
> perform the full hardware update in the .atomic_set_property call chain,
> which isn't what we want to be doing in an atomic driver.
> .atomic_set_property() is just a matter of updating the state structures
> to reflect the changes you *want* to make (but not actually performing
> those changes right away).  It isn't until drm_atomic_commit() gets
> called that we validate the new state structure and then write it to the
> hardware if it looks okay.
>
> Keep in mind that the overall goal behind atomic is that we want to be
> able to supply several items to be updated (e.g., mode, CSC, gamma,
> etc.) via the atomic ioctl and then have them all accepted (and
> programmed) by the driver, or all rejected (so none get programmed) if
> any of them are invalid.  Also, if the collection of properties that
> you're updating fall within the "nuclear pageflip" subset of
> functionality, you'll also get a guarantee that those items all get
> updated within the same vblank; updates that straddle a vblank could
> cause unwanted flickering or other artifacts.  The helper function
> you're updating here only happens to update a single state value at a
> time, but the same .atomic_set_property() is also used by the atomic
> ioctl, so we need to make sure it's following the expected design.
>
> Finally, keep in mind that the function you're updating here is a DRM
> core function.  Even though i915 isn't quite ready for full atomic yet
> and might have a bit of brain damage in areas, other vendors' drivers do
> have complete atomic modesetting support (I think?), so adding
> i915-specific workarounds like this to the core function could actually
> hamper them.
>
>
> Matt
>
I understood this point. But in this case, we will be blocked until set 
CRTC implementation comes in. Can you suggest a better way to handle 
this without getting blocked for set_crtc() ?
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   drivers/gpu/drm/drm_atomic_helper.c | 17 ++---
>>   1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
>> b/drivers/gpu/drm/drm_atomic_helper.c
>> index a900858..37dba55 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1696,6 +1696,8 @@ drm_atomic_helper_crtc_set_property(struct drm_crtc 
>> *crtc,
>>   {
>>  struct drm_atomic_state *state;
>>  struct drm_crtc_state *crtc_state;
>> +struct drm_device *dev = crtc->dev;
>> +struct drm_mode_config *config = &dev->mode_config;
>>  int ret = 0;
>>
>>  state = drm_atomic_state_alloc(crtc->dev);
>> @@ -1716,9 +1718,18 @@ retry:
>>  if (ret)
>>  goto fail;
>>
>> -ret = drm_atomic_commit(state);
>> -if (ret != 0)
>> -goto fail;
>> +/**
>> + * FIXME : This is a hack, to avoid atomic commit
>> + * for CRTC, because i915 supports only
>> + * atomic plane operations at the moment
>> + */
>> +if (property == config->gamma_property)
>> +ret = 0;
>> +else {
>> +ret = drm_atomic_commit(state);
>> +if (ret != 0)
>> +goto fail;
>> +}
>>
>>  /* Driver takes ownership of state on successful commit. */
>>  return 0;
>> --
>> 2.4.2
>>
>

[PATCH v2 05/10] drm: Add a new function for updating color blob

[Sharma, Shashank]

Regards
Shashank

On 6/6/2015 6:30 AM, Matt Roper wrote:
> On Thu, Jun 04, 2015 at 07:12:36PM +0530, Kausal Malladi wrote:
>> From: Kausal Malladi 
>>
>> This patch adds a new function to update color blob properties
>> and exports it.
>>
>> v2: Addressing Sonika's comment,
>> 1. Moved this function to a separate patch
>> 2. Removed one input parameter to the function
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>
> This function is basically just a pass-through.  Can we just un-static
> drm_property_replace_global_blob() so that it can be called directly
> instead?
>
>
> Matt
Yes, sure. In fact we would like to do that, the same comment was given 
by other reviewers. But when we looked at some other examples present in 
the same file (mode_property, edid_property), they have created a 
wrapper function around this, in the same file. So just followed the 
convention, but if that can be done, we will do it in the next patch set.
>
>> ---
>>   drivers/gpu/drm/drm_crtc.c | 15 +++
>>   include/drm/drm_crtc.h |  4 
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 77f87b2..f6fa147 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -4691,6 +4691,21 @@ int drm_mode_connector_set_tile_property(struct 
>> drm_connector *connector)
>>   }
>>   EXPORT_SYMBOL(drm_mode_connector_set_tile_property);
>>
>> +int drm_mode_crtc_update_color_property(struct drm_property_blob **blob,
>> +size_t length, const void *color_data,
>> +struct drm_mode_object *obj_holds_id,
>> +struct drm_property *prop_holds_id)
>> +{
>> +struct drm_device *dev = prop_holds_id->dev;
>> +int ret;
>> +
>> +ret = drm_property_replace_global_blob(dev,
>> +blob, length, color_data, obj_holds_id, prop_holds_id);
>> +
>> +return ret;
>> +}
>> +EXPORT_SYMBOL(drm_mode_crtc_update_color_property);
>> +
>>   /**
>>* drm_mode_connector_update_edid_property - update the edid property of a 
>> connector
>>* @connector: drm connector
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index bc44f27..31b52cb 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1343,6 +1343,10 @@ extern void drm_mode_config_cleanup(struct drm_device 
>> *dev);
>>
>>   extern int drm_mode_connector_set_path_property(struct drm_connector 
>> *connector,
>>  const char *path);
>> +extern int drm_mode_crtc_update_color_property(struct drm_property_blob 
>> **blob,
>> +size_t length, const void *color_data,
>> +struct drm_mode_object *obj_holds_id,
>> +struct drm_property *prop_holds_id);
>>   int drm_mode_connector_set_tile_property(struct drm_connector *connector);
>>   extern int drm_mode_connector_update_edid_property(struct drm_connector 
>> *connector,
>> const struct edid *edid);
>> --
>> 2.4.2
>>
>

[PATCH v2 04/10] drm: Add Gamma correction structure

[Sharma, Shashank]

Regards
Shashank

On 6/6/2015 6:30 AM, Matt Roper wrote:
> On Thu, Jun 04, 2015 at 07:12:35PM +0530, Kausal Malladi wrote:
>> From: Kausal Malladi 
>>
>> This patch adds a new structure in DRM layer for Gamma color correction.
>> This structure will be used by all user space agents to configure
>> appropriate Gamma precision and Gamma level.
>>
>> struct drm_intel_gamma {
>> __u32 gamma_level;
>>  (The gamma_level variable indicates if the Gamma correction is to be
>>  applied on Pipe/plane)
>
> I'm not sure I understand the need for this one...you're getting the
> set_property call against a specific DRM object, so I don't think there
> should be any confusion at that point about what the values apply to?
>
Actually that is for the get_property path. If you have a look at the 
design document, there is a provision to query the current applied gamma 
which can be pipe/plane level. Hence keeping this.
>
>> __u32 gamma_precision;
>>  (The Gamma precision indicates the Gamma mode to be applied)
>>
>>  Supported precisions are -
>>  #define I915_GAMMA_PRECISION_UNKNOWN0
>>  #define I915_GAMMA_PRECISION_CURRENT0x
>>  #define I915_GAMMA_PRECISION_LEGACY (1 << 0)
>>  #define I915_GAMMA_PRECISION_10BIT  (1 << 1)
>>  #define I915_GAMMA_PRECISION_12BIT  (1 << 2)
>>  #define I915_GAMMA_PRECISION_14BIT  (1 << 3)
>>  #define I915_GAMMA_PRECISION_16BIT  (1 << 4)
>
> I feel like the precision would work better as a separate enum property
> rather than being part of your blob; I think it would be cleaner if your
> blob only held the actual values if possible.
>
Again, this would be required for the get_property path. If we create a 
separate property for this, the design might be very complex.

This is the current implementation:
1. Pack the correction values and configurations in blob()
2. call a create_blob() and get the blob_id
3. do a set_porperty() with the blob_id
4. set_property will find this blob, with the blob_id and apply 
corrections.

Adding a separate property for gamma_level will add one more state here, 
which will make it further complex. Do you agree ?
>>
>>  __u32 num_samples;
>>  (The num_samples indicates the number of Gamma correction
>>  coefficients)
>>  __u32 reserved;
>>  __u16 values[0];
>>  (An array of size 0, to accommodate the "num_samples" number of
>>  R16G16B16 pixels, dynamically)
>> };
>>
>> v2: Addressing Daniel Stone's comment, added a variable sized array to
>> carry Gamma correction values as blob property.
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   include/drm/drm_crtc.h |  3 +++
>>   include/uapi/drm/drm.h | 10 ++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 2a75d7d..bc44f27 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -483,6 +483,9 @@ struct drm_crtc {
>>   * acquire context.
>>   */
>>  struct drm_modeset_acquire_ctx *acquire_ctx;
>> +
>> +/* Color Management Blob IDs */
>> +u32 gamma_blob_id;
>>   };
>>
>>   /**
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 3801584..fc2661c 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -829,6 +829,16 @@ struct drm_event_vblank {
>>  __u32 reserved;
>>   };
>>
>> +/* Color Management structure for Gamma */
>> +struct drm_gamma {
>> +__u32 flags;
>
> The flags aren't described in your commit message...what are they used
> for?  I guess it will become more clear as I read farther through your
> patch series.
>
>
> Matt
We are currently using this to define gamma/degamma as such, but yes I 
agree that we have added this for future / undefined last minute usages, 
and can be removed.
>
>
>> +__u32 gamma_level;
>> +__u32 gamma_precision;
>> +__u32 num_samples;
>> +__u32 reserved;
>> +__u16 values[0];
>> +};
>> +
>>   /* typedef area */
>>   #ifndef __KERNEL__
>>   typedef struct drm_clip_rect drm_clip_rect_t;
>> --
>> 2.4.2
>>
>

[PATCH v2 01/10] drm/i915: Initialize Color Manager

[Sharma, Shashank]

Thanks for your time and review Matt.
Please find my comments inline

Regards
Shashank
On 6/6/2015 6:30 AM, Matt Roper wrote:
> On Thu, Jun 04, 2015 at 07:12:32PM +0530, Kausal Malladi wrote:
>> From: Kausal Malladi 
>>
>> Color Manager is an extension in i915 driver to handle color correction
>> and enhancements across various Intel platforms.
>>
>> This patch initializes color manager framework by :
>> 1. Adding two new files, intel_color_manager(.c/.h)
>> 2. Introducing new pointers in DRM mode_config structure to
>> carry CSC and Gamma color correction properties.
>> 3. Creating these DRM properties in Color Manager initialization
>> sequence.
>>
>> v2: Addressing Sonika's review comment.
>> 1. Made intel_color_manager_init void
>> 2. Moved init after NUM_PIPES check
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   drivers/gpu/drm/i915/Makefile  |  3 ++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 48 
>> ++
>>   drivers/gpu/drm/i915/intel_color_manager.h | 32 
>>   drivers/gpu/drm/i915/intel_display.c   |  3 ++
>>   include/drm/drm_crtc.h |  4 +++
>>   5 files changed, 90 insertions(+)
>>   create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index b7ddf48..c62d048 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -89,6 +89,9 @@ i915-y += i915_vgpu.o
>>   # legacy horrors
>>   i915-y += i915_dma.o
>>
>> +# Color Management
>> +i915-y += intel_color_manager.o
>> +
>>   obj-$(CONFIG_DRM_I915)  += i915.o
>>
>>   CFLAGS_i915_trace_points.o := -I$(src)
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> new file mode 100644
>> index 000..f7e2380
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -0,0 +1,48 @@
>> +/*
>> + * Copyright © 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Shashank Sharma 
>> + * Kausal Malladi 
>> + */
>> +
>> +#include "intel_color_manager.h"
>> +
>> +void intel_color_manager_init(struct drm_device *dev)
>> +{
>> +struct drm_mode_config *config = &dev->mode_config;
>> +
>> +/* Create Gamma and CSC properties */
>> +config->gamma_property = drm_property_create(dev,
>> +DRM_MODE_PROP_BLOB, "gamma_property", 0);
>> +if (!config->gamma_property)
>> +DRM_ERROR("Gamma property creation failed\n");
>> +else
>> +DRM_DEBUG_DRIVER("Created Gamma property\n");
>> +
>> +config->csc_property = drm_property_create(dev,
>> +DRM_MODE_PROP_BLOB, "csc_property", 0);
>> +if (!config->csc_property)
>> +DRM_ERROR("CSC property creation failed\n");
>> +else
>> +DRM_DEBUG_DRIVER("Created CSC property\n");
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h 
>> b/drivers/gpu/drm/i915/intel_color_manager.h
>> new file mode 100644
>> index 000..154bf16
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Copyright © 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The

[PATCH 49/88] drm/amdgpu: remove AMDGPU_GEM_CREATE_CPU_GTT_UC

[Oded Gabbay]

Hi Alex,
I think you have a mistake in this patch.
You renamed AMDGPU_GEM_CREATE_CPU_GTT_WC to AMDGPU_GEM_CREATE_CPU_GTT_USWC,
however, AMDGPU_GEM_CREATE_CPU_GTT_WC was defined as (1 << 3) and
AMDGPU_GEM_CREATE_CPU_GTT_USWC is defined as (1 << 2)

Oded

On Wed, May 27, 2015 at 6:22 AM Alex Deucher  wrote:

> From: Jammy Zhou 
>
> This flag isn't used by user mode drivers, remove it to avoid
> confusion. And rename GTT_WC to GTT_USWC to make it clear.
>
> Signed-off-by: Jammy Zhou 
> Reviewed-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++
>  include/uapi/drm/amdgpu_drm.h  |  7 ++-
>  2 files changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index f5e17f9..992b7f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -132,10 +132,7 @@ void amdgpu_ttm_placement_from_domain(struct
> amdgpu_bo *rbo, u32 domain)
> }
>
> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
> -   rbo->placements[c].fpfn = 0;
> -   rbo->placements[c++].flags = TTM_PL_FLAG_UNCACHED
> | TTM_PL_FLAG_TT;
> -   } else if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_WC) {
> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
> rbo->placements[c].fpfn = 0;
> rbo->placements[c++].flags = TTM_PL_FLAG_WC |
> TTM_PL_FLAG_TT |
>
>TTM_PL_FLAG_UNCACHED;
> @@ -146,10 +143,7 @@ void amdgpu_ttm_placement_from_domain(struct
> amdgpu_bo *rbo, u32 domain)
> }
>
> if (domain & AMDGPU_GEM_DOMAIN_CPU) {
> -   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_UC) {
> -   rbo->placements[c].fpfn = 0;
> -   rbo->placements[c++].flags =  TTM_PL_FLAG_UNCACHED
> | TTM_PL_FLAG_SYSTEM;
> -   } else if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_WC) {
> +   if (rbo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) {
> rbo->placements[c].fpfn = 0;
> rbo->placements[c++].flags = TTM_PL_FLAG_WC |
> TTM_PL_FLAG_SYSTEM |
>
>TTM_PL_FLAG_UNCACHED;
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 9e771fb..77bc574 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -73,15 +73,12 @@
>  #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED  (1 << 0)
>  /* Flag that CPU access will not work, this VRAM domain is invisible */
>  #define AMDGPU_GEM_CREATE_NO_CPU_ACCESS(1 << 1)
> -/* Flag that un-cached attributes should be used for GTT */
> -#define AMDGPU_GEM_CREATE_CPU_GTT_UC   (1 << 2)
>  /* Flag that USWC attributes should be used for GTT */
> -#define AMDGPU_GEM_CREATE_CPU_GTT_WC   (1 << 3)
> +#define AMDGPU_GEM_CREATE_CPU_GTT_USWC (1 << 2)
>
>  /* Flag mask for GTT domain_flags */
>  #define AMDGPU_GEM_CREATE_CPU_GTT_MASK \
> -   (AMDGPU_GEM_CREATE_CPU_GTT_WC | \
> -AMDGPU_GEM_CREATE_CPU_GTT_UC | \
> +   (AMDGPU_GEM_CREATE_CPU_GTT_USWC | \
>  AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | \
>  AMDGPU_GEM_CREATE_NO_CPU_ACCESS)
>
> --
> 1.8.3.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/7a94b66b/attachment-0001.html>

[PATCH v2 07/10] drm/i915: Add pipe level Gamma correction for CHV/BSW

[Jindal, Sonika]

On 6/4/2015 7:12 PM, Kausal Malladi wrote:
> From: Kausal Malladi 
>
> This patch does the following:
> 1. Adds the core function to program Gamma correction values for CHV/BSW
> platform
> 2. Adds Gamma correction macros/defines
> 3. Adds drm_mode_crtc_update_color_property function, which replaces the
> old blob for the property with the new one
> 4. Adds a pointer to hold blob for Gamma property in drm_crtc
>
> v2: Addressed all review comments from Sonika and Daniel Stone.
Instead you can mention the changes briefly.

>
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>   drivers/gpu/drm/i915/intel_atomic.c|   6 ++
>   drivers/gpu/drm/i915/intel_color_manager.c | 161 
> +
>   drivers/gpu/drm/i915/intel_color_manager.h |  62 +++
>   3 files changed, 229 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index b5c78d8..4726847 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -428,6 +428,12 @@ intel_crtc_atomic_set_property(struct drm_crtc *crtc,
>  struct drm_property *property,
>  uint64_t val)
>   {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (property == config->gamma_property)
> + return intel_color_manager_set_gamma(dev, &crtc->base, val);
> +
>   DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>   return -EINVAL;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 8d4ee8f..421c267 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,167 @@
>
>   #include "intel_color_manager.h"
>
> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
> + struct drm_crtc *crtc)
> +{
> + struct drm_gamma *gamma_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_property_blob *blob;
> + struct drm_mode_config *config = &dev->mode_config;
> + u32 cgm_control_reg = 0;
> + u32 cgm_gamma_reg = 0;
> + u32 reg, val;
> + enum pipe pipe;
> + u16 red, green, blue;
> + struct rgb_pixel correct_rgb;
> + u32 count = 0;
> + struct rgb_pixel *correction_values = NULL;
> + u32 num_samples;
> + u32 word;
> + u32 palette;
> + int ret = 0, length;
> +
> + blob = drm_property_lookup_blob(dev, blob_id);
> + if (!blob) {
> + DRM_ERROR("Invalid Blob ID\n");
> + return -EINVAL;
> + }
> +
> + gamma_data = (struct drm_gamma *)blob->data;
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = gamma_data->num_samples;
> + length = num_samples * sizeof(struct rgb_pixel);
> +
> + if (gamma_data->gamma_precision == I915_GAMMA_PRECISION_UNKNOWN) {
Switch/case instead?

Also, is UNKNOWN for disabling? Why not rename it to DISABLE then?
> +
> + /* Disable Gamma functionality on Pipe - CGM Block */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_GAMMA_EN;
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> + DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
> + pipe_name(pipe));
> + ret = 0;
> + } else if (gamma_data->gamma_precision == I915_GAMMA_PRECISION_LEGACY) {
> +
> + if (num_samples != CHV_8BIT_GAMMA_MAX_VALS) {
> + DRM_ERROR("Incorrect number of samples received\n");
> + return -EINVAL;
> + }
> +
> + /* First, disable CGM Gamma, if already set */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_GAMMA_EN;
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> + /* Enable (Legacy) Gamma on Pipe */
> + palette = _PIPE_GAMMA_BASE(pipe);
> +
> + count = 0;
> + correction_values = (struct rgb_pixel *)&gamma_data->values;
> + while (count < num_samples) {
> + correct_rgb = correction_values[count];
> + blue = correction_values[count].blue;
> + green = correction_values[count].green;
> + red = correction_values[count].red;
> +
> + blue = blue >> CHV_8BIT_GAMMA_MSB_SHIFT;
> + green = green >> CHV_8BIT_GAMMA_MSB_SHIFT;
> + red = red >> CHV_8BIT_GAMMA_MSB_SHIFT;
> +
> + /* Red (23:16), Green (15:8), Blue (7:0) */
> + word = (red << CHV_8BIT_GAMMA_SHIFT_RED_REG) |
> + (green <<
> +  CHV_8BIT_GAMMA_SHIFT_GREEN_R

[drm fixes] git pull

[Linus Torvalds]

On Fri, Jun 5, 2015 at 11:58 PM, Dave Airlie  wrote:
>
> i915 has a bunch of fixes [..]

.. but nof ix for the regression that Stefan Lippers-Hollmann
reported? An oops in intel_modeset_update_connector_atomic_state() due
to commit 08d9bc920d46 ("drm/i915: Allocate connector state together
with the connectors")?

I hope people are looking at that..

   Linus

[Bug 90873] GPU hang, TearFree On, Mate desktop environment

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=90873

--- Comment #4 from Tom  ---
(In reply to Michel Dänzer from comment #3)
> (In reply to Tom from comment #0)
> > I experience GPU hang while TearFree option is on.
> 
> Does it hang when you do anything in particular?
> 
> FWIW, this line in dmesg is probably related:
> 
> [  141.232226] radeon :01:00.0: bo 8804236d3400 va 0x001000
> conflict with (bo 88042a018800 0x001000 0x001563)
> 
> but I'm not sure what would cause that.
> 
> 
> > I've tested TearFree when this feature was introduced in git version of
> > xf86-video-ati-git and I didn't experience GPU hang back then.
> 
> Can you try isolating the xf86-video-ati commit introducing the hang with
> git bisect?

I investigated it a bit. When I start lightdm it starts fine. I've created new
linux user without any desktop config and it desktop env started also ok. Then
I've changed resolution using desktop environment tool and then problem
appeared. The same happened when used xrandr.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/b4166a43/attachment.html>

[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=73338

--- Comment #91 from Oliver Winker  ---
Ufff - a complicated way to recognize that one day I actually put the mod-param
radeon dpm=0 :-P ... while in the meanwhile dpm=1 actually works ;). 

Anyway, what a nice discovery: The fans are silent now! Great :)) !!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/e24ce7be/attachment.html>

[drm fixes] git pull

[Dave Airlie]

Hi Linus,

i915 has a bunch of fixes, and Russell found a bug in sysfs writing 
handling that results in userspace getting stuck.

Dave.

The following changes since commit 456fdb267377b88fa37abf60fcbd9bfaa813bf1f:

  Merge tag 'pci-v4.1-fixes-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci (2015-06-05 10:22:06 
-0700)

are available in the git repository at:


  git://people.freedesktop.org/~airlied/linux drm-fixes

for you to fetch changes up to 38d8571dad8a759bdc051dbff747b189c90658cf:

  drm: fix writing to /sys/class/drm/*/status (2015-06-06 08:27:30 +1000)


Ander Conselvan de Oliveira (1):
  drm/i915: Include G4X/VLV/CHV in self refresh status

Arun Siluvery (1):
  drm/i915: Initialize HWS page address after GPU reset

Dave Airlie (1):
  Merge tag 'drm-intel-fixes-2015-06-05' of 
git://anongit.freedesktop.org/drm-intel into drm-fixes

Jim Bride (1):
  drm/i915/hsw: Fix workaround for server AUX channel clock divisor

Russell King (1):
  drm: fix writing to /sys/class/drm/*/status

Ville Syrjälä (2):
  drm/i915: Don't skip request retirement if the active list is empty
  drm/i915: Move WaBarrierPerformanceFixDisable:skl to skl code from chv 
code

 drivers/gpu/drm/drm_sysfs.c |  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c |  5 -
 drivers/gpu/drm/i915/i915_gem.c |  3 ---
 drivers/gpu/drm/i915/intel_dp.c |  5 ++---
 drivers/gpu/drm/i915/intel_lrc.c|  6 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 14 +++---
 6 files changed, 20 insertions(+), 15 deletions(-)

[PATCHv2 45/45] drm: omapdrm: add lock for fb pinning

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:03:02 Tomi Valkeinen wrote:
> Before atomic modesetting omap_framebuffer_pin() and
> omap_framebuffer_unpin() were always called with modesetting locks
> taken. With atomic modesetting support this is no longer the case, and
> we need locking to protect the pin_count and the paddr, as multiple
> threads may pin the same fb concurrently.
> 
> This patch adds a mutex to struct omap_framebuffer, and uses it in
> omap_framebuffer_pin() and omap_framebuffer_unpin().
> 
> Signed-off-by: Tomi Valkeinen 

Acked-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_fb.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
> b/drivers/gpu/drm/omapdrm/omap_fb.c index e505140a8782..0b967e76df1a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -89,6 +89,8 @@ struct omap_framebuffer {
>   int pin_count;
>   const struct format *format;
>   struct plane planes[4];
> + /* lock for pinning (pin_count and planes.paddr) */
> + struct mutex lock;
>  };
> 
>  static int omap_framebuffer_create_handle(struct drm_framebuffer *fb,
> @@ -250,8 +252,11 @@ int omap_framebuffer_pin(struct drm_framebuffer *fb)
>   struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
>   int ret, i, n = drm_format_num_planes(fb->pixel_format);
> 
> + mutex_lock(&omap_fb->lock);
> +
>   if (omap_fb->pin_count > 0) {
>   omap_fb->pin_count++;
> + mutex_unlock(&omap_fb->lock);
>   return 0;
>   }
> 
> @@ -265,6 +270,8 @@ int omap_framebuffer_pin(struct drm_framebuffer *fb)
> 
>   omap_fb->pin_count++;
> 
> + mutex_unlock(&omap_fb->lock);
> +
>   return 0;
> 
>  fail:
> @@ -274,6 +281,8 @@ fail:
>   plane->paddr = 0;
>   }
> 
> + mutex_unlock(&omap_fb->lock);
> +
>   return ret;
>  }
> 
> @@ -283,10 +292,14 @@ int omap_framebuffer_unpin(struct drm_framebuffer *fb)
> struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
>   int ret, i, n = drm_format_num_planes(fb->pixel_format);
> 
> + mutex_lock(&omap_fb->lock);
> +
>   omap_fb->pin_count--;
> 
> - if (omap_fb->pin_count > 0)
> + if (omap_fb->pin_count > 0) {
> + mutex_unlock(&omap_fb->lock);
>   return 0;
> + }
> 
>   for (i = 0; i < n; i++) {
>   struct plane *plane = &omap_fb->planes[i];
> @@ -296,9 +309,12 @@ int omap_framebuffer_unpin(struct drm_framebuffer *fb)
>   plane->paddr = 0;
>   }
> 
> + mutex_unlock(&omap_fb->lock);
> +
>   return 0;
> 
>  fail:
> + mutex_unlock(&omap_fb->lock);
>   return ret;
>  }
> 
> @@ -411,6 +427,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct
> drm_device *dev,
> 
>   fb = &omap_fb->base;
>   omap_fb->format = format;
> + mutex_init(&omap_fb->lock);
> 
>   for (i = 0; i < n; i++) {
>   struct plane *plane = &omap_fb->planes[i];

-- 
Regards,

Laurent Pinchart

[PATCHv2 43/45] drm: omapdrm: don't wait in crtc_atomic_flush

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:03:00 Tomi Valkeinen wrote:
> omap_crtc_atomic_flush() sets the GO bit and waits for it to get unset,
> which tells us the HW has taken the new configuration into use.
> 
> This is unnecessary as omap_atomic_complete() waits for all the GO bits
> to get unset. In fact, waiting is wrong, as with multiple CRTCs we would
> not get the the changes to all the CRTCs as soon as possible, but only
> one after another.
> 
> This patch removes the wait.
> 
> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 12 
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 8f905d2c8074..2ec34dc0c66c
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -17,8 +17,6 @@
>   * this program.  If not, see .
>   */
> 
> -#include 
> -
>  #include 
>  #include 
>  #include 
> @@ -52,8 +50,6 @@ struct omap_crtc {
>   /* pending event */
>   struct drm_pending_vblank_event *event;
> 
> - struct completion completion;
> -
>   bool ignore_digit_sync_lost;
>  };
> 
> @@ -317,8 +313,6 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq
> *irq, uint32_t irqstatus)
> 
>   /* wakeup userspace */
>   omap_crtc_complete_page_flip(&omap_crtc->base);
> -
> - complete(&omap_crtc->completion);
>  }
> 
>  static int omap_crtc_flush(struct drm_crtc *crtc)
> @@ -332,10 +326,6 @@ static int omap_crtc_flush(struct drm_crtc *crtc)
>   if (dispc_mgr_is_enabled(omap_crtc->channel)) {
>   dispc_mgr_go(omap_crtc->channel);
>   omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
> -
> - WARN_ON(!wait_for_completion_timeout(&omap_crtc->completion,
> -  msecs_to_jiffies(100)));
> - reinit_completion(&omap_crtc->completion);

I wonder whether this could introduce a race condition between the CRTC vblank 
interrupt handler register here, and the wait for gos in atomic_commit. The 
latter might run before our CRTC vblank interrupt handler, potentially 
starting processing of the next commit with vblank_irq still registered.

Bonus points if you can remove vblank_irq completely while fixing that :-) I'd 
rather see omap_crtc_vblank_irq() being called directly and unconditionally 
from omap_irq_handler(), and the drm_crtc_handle_vblank() call being moved to 
omap_crtc_vblank_irq().

>   }
> 
>   return 0;
> @@ -517,8 +507,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
> 
>   crtc = &omap_crtc->base;
> 
> - init_completion(&omap_crtc->completion);
> -
>   omap_crtc->channel = channel;
>   omap_crtc->name = channel_names[channel];

-- 
Regards,

Laurent Pinchart

[GIT PULL] tilcdc DRM refactoring (again)

[Dave Airlie]

On 6 June 2015 at 00:05, Jyri Sarha  wrote:
> David,
> Please pull the contents of "Use DRM component API in tilcdc to
> connect to tda998x" patch series[1] from:
>
> https://github.com/jsarha/linux.git linux-4.1.0-rc6-tilcdc-refactor

I think I pulled the earlier version a few days ago,

Have a look in drm-next.

Dave.

[PATCHv2 42/45] drm: omapdrm: add omap_atomic_wait_for_gos()

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:02:59 Tomi Valkeinen wrote:
> omap_atomic_complete() uses drm_atomic_helper_wait_for_vblanks() to wait
> for all operations to finish. That works, but can easily cause waits for
> vblanks when no wait is actually necessary.

It actually doesn't work properly, as a full mode set without any change to 
framebuffers will need to reenable planes after enabling the CRTC and thus 
sets the go bit, but the drm_atomic_helper_wait_for_vblanks() function called 
during atomic commit waits for vblank only on CRTCs that have seen a page 
flip. This leads to missing waits, resulting in a go busy warning if the next 
.atomic_flush() call occurs too soon.

Your patch fixes that issue, so I'm fine with the code, but I think the commit 
message should be fixed. Then,

Acked-by: Laurent Pinchart 

> This patch adds omap_atomic_wait_for_gos() and uses it instead.
> omap_atomic_wait_for_gos() waits for the GO bit to get unset for all
> relevant crtcs.
> 
> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c |  7 +++
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 31 ++-
>  drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2c0d91a67418..8f905d2c8074
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -80,6 +80,13 @@ enum omap_channel omap_crtc_channel(struct drm_crtc
> *crtc) return omap_crtc->channel;
>  }
> 
> +bool omap_crtc_needs_wait(struct drm_crtc *crtc)
> +{
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
> + return dispc_mgr_go_busy(omap_crtc->channel);
> +}
> +
>  /*
> ---
> -- * DSS Manager Functions
>   */
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 50f30e55..c03405593f9f
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -66,6 +66,35 @@ struct omap_atomic_state_commit {
>   u32 crtcs;
>  };
> 
> +static void omap_atomic_wait_for_gos(struct drm_device *dev,
> +  struct drm_atomic_state *old_state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *old_crtc_state;
> + int i, ret;
> +
> + for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
> + if (!crtc->state->enable)
> + continue;
> +
> + if (!omap_crtc_needs_wait(crtc))
> + continue;
> +
> + ret = drm_crtc_vblank_get(crtc);
> + if (ret != 0)
> + continue;
> +
> + ret = wait_event_timeout(dev->vblank[i].queue,
> +  !omap_crtc_needs_wait(crtc),
> +  msecs_to_jiffies(50));
> + if (!ret)
> + dev_warn(dev->dev,
> +  "atomic flush timeout (pipe %u)!\n", i);
> +
> + drm_crtc_vblank_put(crtc);
> + }
> +}
> +
>  static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
>  {
>   struct drm_device *dev = commit->dev;
> @@ -79,7 +108,7 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) drm_atomic_helper_commit_planes(dev,
> old_state);
>   drm_atomic_helper_commit_modeset_enables(dev, old_state);
> 
> - drm_atomic_helper_wait_for_vblanks(dev, old_state);
> + omap_atomic_wait_for_gos(dev, old_state);
> 
>   drm_atomic_helper_cleanup_planes(dev, old_state);
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index 0b7a055bf007..27bbf7c4d76e
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -147,6 +147,7 @@ void omap_crtc_pre_init(void);
>  void omap_crtc_pre_uninit(void);
>  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>   struct drm_plane *plane, enum omap_channel channel, int id);
> +bool omap_crtc_needs_wait(struct drm_crtc *crtc);
> 
>  struct drm_plane *omap_plane_init(struct drm_device *dev,
>   int id, enum drm_plane_type type);

-- 
Regards,

Laurent Pinchart

[PATCHv2 41/45] drm: omapdrm: remove omap_crtc_wait_page_flip

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:02:58 Tomi Valkeinen wrote:
> omap_crtc_disable() waits for pending page flips to finish. However,
> omap_crtc_disable() is always called via omap_atomic_complete() and we
> never have page flips going on at that time.

Why is that ? Because our omap_atomic_complete() implementation waits for 
vblanks before allowing the next atomic commit to run, and that our vblank IRQ 
handler completes all pending page flips ? If so, I believe we have a few 
corner cases that won't work properly.

For instance, if the user performs a full mode set on a CRTC without change 
the framebuffer, an event can be queued but 
drm_atomic_helper_wait_for_vblanks() won't wait for vblank on that CRTC as 
framebuffer_changed() will return false. If the next commit then disables the 
CRTC the event might not have completed yet.

> So remove the omap_crtc_wait_page_flip() and related code.
> 
> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 32 
>  1 file changed, 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index abfafd1600b8..2c0d91a67418
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -51,7 +51,6 @@ struct omap_crtc {
> 
>   /* pending event */
>   struct drm_pending_vblank_event *event;
> - wait_queue_head_t flip_wait;
> 
>   struct completion completion;
> 
> @@ -277,41 +276,12 @@ static void omap_crtc_complete_page_flip(struct
> drm_crtc *crtc) else
>   event->base.destroy(&event->base);
> 
> - wake_up(&omap_crtc->flip_wait);
>   drm_crtc_vblank_put(crtc);
>   }
> 
>   spin_unlock_irqrestore(&dev->event_lock, flags);
>  }
> 
> -static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
> -{
> - struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> - struct drm_device *dev = crtc->dev;
> - unsigned long flags;
> - bool pending;
> -
> - spin_lock_irqsave(&dev->event_lock, flags);
> - pending = omap_crtc->event != NULL;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
> - return pending;
> -}
> -
> -static void omap_crtc_wait_page_flip(struct drm_crtc *crtc)
> -{
> - struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> -
> - if (wait_event_timeout(omap_crtc->flip_wait,
> -!omap_crtc_page_flip_pending(crtc),
> -msecs_to_jiffies(50)))
> - return;
> -
> - dev_warn(crtc->dev->dev, "page flip timeout!\n");
> -
> - omap_crtc_complete_page_flip(crtc);
> -}
> -
>  static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t
> irqstatus) {
>   struct omap_crtc *omap_crtc =
> @@ -404,7 +374,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
> 
>   DBG("%s", omap_crtc->name);
> 
> - omap_crtc_wait_page_flip(crtc);
>   drm_crtc_vblank_off(crtc);
>  }
> 
> @@ -541,7 +510,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
> 
>   crtc = &omap_crtc->base;
> 
> - init_waitqueue_head(&omap_crtc->flip_wait);
>   init_completion(&omap_crtc->completion);
> 
>   omap_crtc->channel = channel;

-- 
Regards,

Laurent Pinchart

[PATCHv2 44/45] drm: omapdrm: merge omap_crtc_flush and omap_crtc_atomic_flush

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:03:01 Tomi Valkeinen wrote:
> omap_crtc_atomic_flush() is the only user of omap_crtc_flush(), so just
> move the code from omap_crtc_flush() to omap_crtc_atomic_flush().
> 
> Signed-off-by: Tomi Valkeinen 

Acked-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 27 ++-
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2ec34dc0c66c..b7df689cdb4c
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -315,22 +315,6 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq
> *irq, uint32_t irqstatus) omap_crtc_complete_page_flip(&omap_crtc->base);
>  }
> 
> -static int omap_crtc_flush(struct drm_crtc *crtc)
> -{
> - struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> -
> - DBG("%s: GO", omap_crtc->name);
> -
> - WARN_ON(omap_crtc->vblank_irq.registered);
> -
> - if (dispc_mgr_is_enabled(omap_crtc->channel)) {
> - dispc_mgr_go(omap_crtc->channel);
> - omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
> - }
> -
> - return 0;
> -}
> -
>  /*
> ---
> -- * CRTC Functions
>   */
> @@ -408,7 +392,16 @@ static void omap_crtc_atomic_begin(struct drm_crtc
> *crtc)
> 
>  static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
>  {
> - omap_crtc_flush(crtc);
> + struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
> + if (dispc_mgr_is_enabled(omap_crtc->channel)) {
> + WARN_ON(omap_crtc->vblank_irq.registered);
> +
> + DBG("%s: GO", omap_crtc->name);
> +
> + dispc_mgr_go(omap_crtc->channel);
> + omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
> + }
> 
>   crtc->invert_dimensions = !!(crtc->primary->state->rotation &
>   (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270)));

-- 
Regards,

Laurent Pinchart

[PATCHv2 40/45] drm: omapdrm: if omap_plane_atomic_update fails, disable plane

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:02:57 Tomi Valkeinen wrote:
> omap_plane_atomic_update() calls dispc_ovl_setup(), which can fail (but
> shouldn't). To make the code a bit more robust, make sure the plane gets
> disabled if dispc_ovl_setup() fails, as otherwise we might get illegal
> HW configuration leading to error interrupts.
> 
> Signed-off-by: Tomi Valkeinen 

Acked-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index b13fb2fd4a9a..cfa8276c4deb
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -132,8 +132,10 @@ static void omap_plane_atomic_update(struct drm_plane
> *plane, /* and finally, update omapdss: */
>   ret = dispc_ovl_setup(omap_plane->id, &info, false,
> omap_crtc_timings(state->crtc), false);
> - if (WARN_ON(ret))
> + if (WARN_ON(ret)) {
> + dispc_ovl_enable(omap_plane->id, false);
>   return;
> + }
> 
>   dispc_ovl_enable(omap_plane->id, true);
>  }

-- 
Regards,

Laurent Pinchart

[PATCHv2 39/45] drm: omapdrm: inline omap_plane_setup into update/disable

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:02:56 Tomi Valkeinen wrote:
> At the moment we have omap_plane_setup() function which handles both
> enabling (and configuring) and disabling the plane. With atomic
> modesetting we have separate hooks for plane enable/config and disable.
> 
> This patch moves the code from omap_plane_setup() to
> omap_plane_atomic_update() and omap_plane_atomic_disable().
> 
> Signed-off-by: Tomi Valkeinen 

Acked-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_plane.c | 44 +
>  1 file changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index a8e617f9f2af..b13fb2fd4a9a
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -58,7 +58,22 @@ to_omap_plane_state(struct drm_plane_state *state)
>   return container_of(state, struct omap_plane_state, base);
>  }
> 
> -static void omap_plane_setup(struct drm_plane *plane)
> +static int omap_plane_prepare_fb(struct drm_plane *plane,
> +  struct drm_framebuffer *fb,
> +  const struct drm_plane_state *new_state)
> +{
> + return omap_framebuffer_pin(fb);
> +}
> +
> +static void omap_plane_cleanup_fb(struct drm_plane *plane,
> +   struct drm_framebuffer *fb,
> +   const struct drm_plane_state *old_state)
> +{
> + omap_framebuffer_unpin(fb);
> +}
> +
> +static void omap_plane_atomic_update(struct drm_plane *plane,
> +  struct drm_plane_state *old_state)
>  {
>   struct omap_plane *omap_plane = to_omap_plane(plane);
>   struct drm_plane_state *state = plane->state;
> @@ -69,11 +84,6 @@ static void omap_plane_setup(struct drm_plane *plane)
> 
>   DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
> 
> - if (!state->crtc) {
> - dispc_ovl_enable(omap_plane->id, false);
> - return;
> - }
> -
>   memset(&info, 0, sizeof(info));
>   info.rotation_type = OMAP_DSS_ROT_DMA;
>   info.rotation = OMAP_DSS_ROT_0;
> @@ -128,26 +138,6 @@ static void omap_plane_setup(struct drm_plane *plane)
>   dispc_ovl_enable(omap_plane->id, true);
>  }
> 
> -static int omap_plane_prepare_fb(struct drm_plane *plane,
> -  struct drm_framebuffer *fb,
> -  const struct drm_plane_state *new_state)
> -{
> - return omap_framebuffer_pin(fb);
> -}
> -
> -static void omap_plane_cleanup_fb(struct drm_plane *plane,
> -   struct drm_framebuffer *fb,
> -   const struct drm_plane_state *old_state)
> -{
> - omap_framebuffer_unpin(fb);
> -}
> -
> -static void omap_plane_atomic_update(struct drm_plane *plane,
> -  struct drm_plane_state *old_state)
> -{
> - omap_plane_setup(plane);
> -}
> -
>  static void omap_plane_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
>  {
> @@ -158,7 +148,7 @@ static void omap_plane_atomic_disable(struct drm_plane
> *plane, omap_state->zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
>  ? 0 : omap_plane->id;
> 
> - omap_plane_setup(plane);
> + dispc_ovl_enable(omap_plane->id, false);
>  }
> 
>  static const struct drm_plane_helper_funcs omap_plane_helper_funcs = {

-- 
Regards,

Laurent Pinchart

[PATCHv2 38/45] drm: omapdrm: omap_plane_setup() cannot fail, use WARN

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thursday 04 June 2015 12:02:55 Tomi Valkeinen wrote:
> With atomic modesetting, omap_plane_setup()'s return value is ignored as
> the functions using it cannot fail. dispc_ovl_setup(), called by
> omap_plane_setup(), can fail (but shouldn't).
> 
> Instead of returning an error from omap_plane_setup() which gets
> ignored, return void and use WARN if dispc_ovl_setup() fails.
> 
> Signed-off-by: Tomi Valkeinen 

Acked-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/omap_plane.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c
> b/drivers/gpu/drm/omapdrm/omap_plane.c index 448707669690..a8e617f9f2af
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -58,12 +58,11 @@ to_omap_plane_state(struct drm_plane_state *state)
>   return container_of(state, struct omap_plane_state, base);
>  }
> 
> -static int omap_plane_setup(struct drm_plane *plane)
> +static void omap_plane_setup(struct drm_plane *plane)
>  {
>   struct omap_plane *omap_plane = to_omap_plane(plane);
>   struct drm_plane_state *state = plane->state;
>   struct omap_plane_state *omap_state = to_omap_plane_state(state);
> - struct drm_device *dev = plane->dev;
>   struct omap_overlay_info info;
>   struct omap_drm_window win;
>   int ret;
> @@ -72,7 +71,7 @@ static int omap_plane_setup(struct drm_plane *plane)
> 
>   if (!state->crtc) {
>   dispc_ovl_enable(omap_plane->id, false);
> - return 0;
> + return;
>   }
> 
>   memset(&info, 0, sizeof(info));
> @@ -123,14 +122,10 @@ static int omap_plane_setup(struct drm_plane *plane)
>   /* and finally, update omapdss: */
>   ret = dispc_ovl_setup(omap_plane->id, &info, false,
> omap_crtc_timings(state->crtc), false);
> - if (ret) {
> - dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
> - return ret;
> - }
> + if (WARN_ON(ret))
> + return;
> 
>   dispc_ovl_enable(omap_plane->id, true);
> -
> - return 0;
>  }
> 
>  static int omap_plane_prepare_fb(struct drm_plane *plane,

-- 
Regards,

Laurent Pinchart

[Bug 73338] Fan speed in idle at 40% with radeonsi and at 18% with catalyst

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=73338

--- Comment #90 from Oliver Winker  ---
In fact it seems that controlling the fans on my card using the ADL Overdrive6
api doesn't work. 

But it works using Overdrive5. When I modify Olegs test-program to use
ADL_Overdrive5_FanSpeed[Get|Set|ToDefault_Set]() api indeed my fans react.

So the next thing I guess would be to check the mmio-traces. In fact for me
personally already the equivalent of ADL_Overdrive5_FanSpeedToDefault_Set()
would be sufficient, since it seems to put the fan automagically to a good
minimum.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/a91f95cf/attachment.html>

[Bug 90873] GPU hang, TearFree On, Mate desktop environment

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=90873

--- Comment #3 from Michel Dänzer  ---
(In reply to Tom from comment #0)
> I experience GPU hang while TearFree option is on.

Does it hang when you do anything in particular?

FWIW, this line in dmesg is probably related:

[  141.232226] radeon :01:00.0: bo 8804236d3400 va 0x001000
conflict with (bo 88042a018800 0x001000 0x001563)

but I'm not sure what would cause that.


> I've tested TearFree when this feature was introduced in git version of
> xf86-video-ati-git and I didn't experience GPU hang back then.

Can you try isolating the xf86-video-ati commit introducing the hang with git
bisect?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20150606/5eed0f79/attachment.html>

[PATCH v3 1/2] drm/bridge: dw-hdmi: support optional supply regulators

[Heiko Stübner]

Hi Thierry

Am Freitag, 5. Juni 2015, 14:23:14 schrieb Thierry Reding:
> On Fri, Jun 05, 2015 at 02:16:40PM +0200, Heiko Stübner wrote:
> > > If this is specific to the Rockchip implementation, shouldn't this go
> > > into Documentation/devicetree/bindings/video/dw_hdmi-rockchip.txt? It
> > > could then simply go into the Rockchip DRM tree.
> > 
> > actually, we determined that the supply names are universal to the IP
> > (both in imx and rockchip and probably more if there are more users out
> > there). Just Russell requested that we don't pollute the generic code
> > until necessary, as it looks like the supply of those is somehow handled
> > internally on the imx.
> If it's universal then there should be no need to mention the Rockchip
> compatible specifically. Also, it might be better to submit this as two
> separate patches, one for the binding and another for the driver.
> 
> I could extract the DT binding piece myself and apply only that, then
> somebody else can apply the Rockchip change to that driver separately.

so I've sent a v4 where the dt-binding is split off (and without the Rockchip 
specific text), so if you want to take patch1, Mark could take patch2 and I 
would take the dts changes in patch3.


Heiko

[PATCH v3 1/2] drm/bridge: dw-hdmi: support optional supply regulators

[Russell King - ARM Linux]

On Fri, Jun 05, 2015 at 02:16:40PM +0200, Heiko Stübner wrote:
> Hi Thierry
> 
> Am Freitag, 5. Juni 2015, 13:02:01 schrieb Thierry Reding:
> > If this is specific to the Rockchip implementation, shouldn't this go
> > into Documentation/devicetree/bindings/video/dw_hdmi-rockchip.txt? It
> > could then simply go into the Rockchip DRM tree.
> 
> actually, we determined that the supply names are universal to the IP
> (both in imx and rockchip and probably more if there are more users
> out there). Just Russell requested that we don't pollute the generic
> code until necessary, as it looks like the supply of those is somehow
> handled internally on the imx.

Why do you think it's universal?

Let's start from the beginning, before we create something that's not
representative of the hardware.

dw_hdmi actually drives two pieces of hardware - the HDMI transmitter,
and a separate hardware block, the HDMI phy.

There are at least two possible HDMI phys referenced in the iMX
documentation - there is one called HEAC which doesn't appear in iMX
devices afaik, and the one which does, which is a 3D phy.

The 3D phy top level IO diagram does have VPH and VP power supplies,
but it also has VP_FILT0, VP_FILT1, VP_FILT2, VP_FILT3 and GD labelled
up as "Power supply signals".  Where they connect to in the rest of the
system - or whether they are connected - is undocumented.

The HDMI transmitter itself is not documented what it's supplies
actually are.

So, what we currently have in DT for dw_hdmi is something which doesn't
_quite_ reflect the hardware right now, and to go adding VP and VPH
supplies to the generic binding is wrong - it doesn't follow the
hardware structure detailed in the iMX documentation I have.

As the Rockchip documentation is not available, I can't comment whether
it would be current proposal is appropriate for Rockchip or not, which
is why I haven't commented on this other than saying that it's not
appropriate to be a generic dw_hdmi binding.

If we wanted to model this correctly, then for iMX, I would suggest
that the HDMI phy should be modelled in DT, and _all_ the six VP*
supplies are modelled - and we should assume that "GD" is a "power
good" signal, though we don't know that for certain.

What we also don't know on iMX6 is what voltages any of these are
supplied with.

So, as we don't have much certainty here, and we know that adding it
to what is the HDMI transmitter would be wrong, I'd suggest not
modelling it in a generic way at present.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.