[PATCH 0/5] Add ASoC support for AMD APUs [v3]

[Christian König]

Only briefly skimmed over the patches, but the approach sounds sane to me.

So the set is Acked-by: Christian König 

Regards,
Christian.

On 23.09.2015 19:02, Alex Deucher wrote:
> This patch set implements support for i2s audio and new AMD GPUs.
> The i2s codec is fed by a DMA engine on the GPU.  To handle this
> we create mfd cells which we hang the i2s codec and DMA engine on.
> Because of this, this patch set covers two subsystems: drm and alsa.
> The drm patches add support for the ACP hw block which provides the
> DMA engine for the i2s codec.  The alsa patches add the ASoC driver
> for the i2s codec.  Since the alsa changes depend on the drm changes
> in this patch set as well as some other drm changes queued for 4.3,
> I'd like to take the alsa patches in via the drm tree.
>
> V2 changes:
> - Use the MFD subsystem rather than adding our own bus
> - Squash all sub-feature patches together
> - fix comments mentioned in previous review
>
> V3 changes:
> - Update the designware driver to handle slave mode, amd specific
>features
> - Use the designware driver directly for i2s
> - Move the DMA handling from the GPU driver into the AMD ASoC
>driver
> - Change the license on the ASoC driver to GPL
>
> Patch 4 adds the register headers for the ACP block which is a
> pretty big patch so I've excluded it from email.  The entire patch
> set can be viewed here:
> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acp-upstream3
>
> Thanks,
>
> Alex
>
> Maruthi Bayyavarapu (1):
>drm/amd: add ACP driver support
>
> Maruthi Srinivas Bayyavarapu (4):
>ASoC : dwc : support dw i2s in slave mode
>ASoC : dwc : support dw i2s in AMD platform
>ASoC : AMD : add ACP 2.2 register headers
>ASoC: AMD: add AMD ASoC ACP-I2S driver
>
>   drivers/gpu/drm/Kconfig  |2 +
>   drivers/gpu/drm/amd/acp/Kconfig  |   10 +
>   drivers/gpu/drm/amd/acp/Makefile |9 +
>   drivers/gpu/drm/amd/acp/acp_hw.c |  127 ++
>   drivers/gpu/drm/amd/acp/include/acp_gfx_if.h |   49 +
>   drivers/gpu/drm/amd/amdgpu/Makefile  |   13 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h  |   12 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c  |  269 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h  |   41 +
>   drivers/gpu/drm/amd/amdgpu/vi.c  |   12 +
>   drivers/gpu/drm/amd/include/amd_shared.h |1 +
>   include/linux/mfd/amd_acp.h  |   43 +
>   include/sound/designware_i2s.h   |4 +
>   sound/soc/Kconfig|1 +
>   sound/soc/Makefile   |1 +
>   sound/soc/amd/Kconfig|4 +
>   sound/soc/amd/Makefile   |3 +
>   sound/soc/amd/acp-pcm-dma.c  |  518 ++
>   sound/soc/amd/acp.c  |  736 +
>   sound/soc/amd/acp.h  |  147 ++
>   sound/soc/amd/include/acp_2_2_d.h|  609 +++
>   sound/soc/amd/include/acp_2_2_enum.h | 1068 
>   sound/soc/amd/include/acp_2_2_sh_mask.h  | 2292 
> ++
>   sound/soc/dwc/designware_i2s.c   |  270 +--
>   24 files changed, 6141 insertions(+), 100 deletions(-)
>   create mode 100644 drivers/gpu/drm/amd/acp/Kconfig
>   create mode 100644 drivers/gpu/drm/amd/acp/Makefile
>   create mode 100644 drivers/gpu/drm/amd/acp/acp_hw.c
>   create mode 100644 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h
>   create mode 100644 include/linux/mfd/amd_acp.h
>   create mode 100644 sound/soc/amd/Kconfig
>   create mode 100644 sound/soc/amd/Makefile
>   create mode 100644 sound/soc/amd/acp-pcm-dma.c
>   create mode 100644 sound/soc/amd/acp.c
>   create mode 100644 sound/soc/amd/acp.h
>   create mode 100644 sound/soc/amd/include/acp_2_2_d.h
>   create mode 100644 sound/soc/amd/include/acp_2_2_enum.h
>   create mode 100644 sound/soc/amd/include/acp_2_2_sh_mask.h
>

drm_device from another device driver? (was: Re: block device backed by DRM buffer object)

[Steven Newbury]

I've been reading up on the device model and studying kernel sources for the 
last couple of days, but I can't figure out how to get a pointer to the 
radeon_device struct for a specific card, or the parent drm_device from an 
external device driver.

I imagine I somehow need to take a reference to the drm class kobject for the 
card in question, and in so doing presumably I should then be able to discover 
the pointer to device.

Can someone point me to an example or provide a snippet to demonstrate?

Alternatively, if I'm wandering down the wrong path, a sign to lead my way 
would be appreciated!

[PATCH] drm/radeon: Sprinkle drm_modeset_lock_all to appease locking checks

[Daniel Vetter]

In

commit 7a3f3d6667f5f9ffd1517f6b21d64bbf5312042c
Author: Daniel Vetter 
Date:   Thu Jul 9 23:44:28 2015 +0200

drm: Check locking in drm_for_each_connector

I added locking checks to drm_for_each_connector but failed that
through drm_helper_connector_dpms -> drm_helper_choose_encoder_dpms
it's used in a few more places in the radeon resume/suspend code.

Fix them up.

Note that we could use the connector iterator macros in there too, but
that's for the future.

Reported-and-tested-by: Borislav Petkov 
Cc: Borislav Petkov 
Cc: Alex Deucher 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/radeon/radeon_device.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index d8319dae8358..f3f562f6d848 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1573,10 +1573,12 @@ int radeon_suspend_kms(struct drm_device *dev, bool 
suspend, bool fbcon)

drm_kms_helper_poll_disable(dev);

+   drm_modeset_lock_all(dev);
/* turn off display hw */
list_for_each_entry(connector, >mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
}
+   drm_modeset_unlock_all(dev);

/* unpin the front buffers and cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
@@ -1734,9 +1736,11 @@ int radeon_resume_kms(struct drm_device *dev, bool 
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
+   drm_modeset_lock_all(dev);
list_for_each_entry(connector, 
>mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
+   drm_modeset_unlock_all(dev);
}

drm_kms_helper_poll_enable(dev);
-- 
2.5.1

[PATCH] drm/core: remove unused variable

[Sudip Mukherjee]

The variable dev was not used anywhere.

Signed-off-by: Sudip Mukherjee 
---
 drivers/gpu/drm/drm_crtc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9d55c0c..e600a5f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3491,7 +3491,6 @@ out_err1:
  */
 void drm_fb_release(struct drm_file *priv)
 {
-   struct drm_device *dev = priv->minor->dev;
struct drm_framebuffer *fb, *tfb;

/*
-- 
1.9.1

[PATCH 04/23] drm: Add drm structures for palette color property

[Sharma, Shashank]

Regards
Shashank

On 9/23/2015 6:19 PM, Daniel Vetter wrote:
> On Wed, Sep 23, 2015 at 01:45:16PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 9/22/2015 6:38 PM, Daniel Vetter wrote:
>>> On Wed, Sep 16, 2015 at 11:07:01PM +0530, Shashank Sharma wrote:
 From: Kausal Malladi 

 This patch adds new structures in DRM layer for Palette color
 correction.These structures will be used by user space agents
 to configure appropriate number of samples and Palette LUT for
 a platform.

 Signed-off-by: Shashank Sharma 
 Signed-off-by: Kausal Malladi 
 ---
   include/uapi/drm/drm.h | 27 +++
   1 file changed, 27 insertions(+)

 diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
 index e3c642f..f72b916 100644
 --- a/include/uapi/drm/drm.h
 +++ b/include/uapi/drm/drm.h
 @@ -840,6 +840,33 @@ struct drm_palette_caps {
__u32 num_samples_after_ctm;
   };

 +struct drm_r32g32b32 {
 +  /*
 +   * Data is in U8.24 fixed point format.
 +   * All platforms support values within [0, 1.0] range,
 +   * for Red, Green and Blue colors.
 +   */
 +  __u32 r32;
 +  __u32 g32;
 +  __u32 b32;
>>>
>>> It's not strictly required, but adding a __u32 reserved here to align the
>>> struct to 64 bits seems good imo. Slight overhead but meh about that.
>> Humm, ok, we can check this out.
>>>
 +};
 +
 +struct drm_palette {
 +  /* Structure version. Should be 1 currently */
 +  __u32 version;
>>>
>>> Definitely great practice to take compat into account and definitely
>>> needed for the first design using ioctls but I don't think we need this
>>> here. Properties are already extinsible themselves: We can just greate a
>>> "ctm-v2", "ctm-v3" if the layout changes, and since the actual ctm matrix
>>> is stored in the drm_crtc_state any compat code on the kernel will be
>>> shared.
>>>
>>> Aside: For an ioctl the recommended way to handle backwards compat and
>>> extensions in drm is with a flags bitfield. That's more flexible than a
>>> linear version field, and extending the ioctl struct at the end is already
>>> handled by the drm core in a transparent fashion (it 0-fills either kernel
>>> or userspace side).
>>>
>> Agree, we will drop this. Do you think we should add a flags field, or is it
>> ok without it ?
>
> No need for a flag field since this is not an ioctl struct. That "Aside:"
> was really meant as a comment aside and not relevant for properties.
>
 +  /*
 +   * This has to be a supported value during get call.
 +   * Feature will be disabled if this is 0 while set
 +   */
 +  __u32 num_samples;
>>>
>>> blob properties already have a size, storing it again in the blob is
>>> redundnant. Instead I think a small helper to get the number of samples
>>> for a given gamma table blob would be needed.
>>>
>>> Cheers, Daniel
>> Please note that they are different. One is the size of blob and other one
>> is the num_samples supported by the property, in the current correction
>> mode. If you check the design doc, num_sample serves the purpose of deciding
>> which correction mode to be applied also. fox ex, for gamma, num_samples=0
>> indicates disable gamma, whereas num_samples=512 indicates split gamma mode.
>
> num_samples = blob_size/(sizeof(drm_r32g32b32));
>
> I just think that this information is redundant and if userspace supplies
> a gamma table with the wrong size we should just reject it. There's really
> no reason for userspace to create a blob property where the size doesn't
> exactly match the gamma table.
>
> I guess again that this was needed for the ioctl where there's no sideband
> for the size. But properties _are_ sized.
Again, this is what we decided in the design discussion. The driver will 
showcase the best option for property, but that doesn't stop a user 
space with more knowledge of HW to send other supported options. for 
example, in case of gamma, the driver supports all 3 possible modes:
- 8 bit legacy gamma (256 coeff)
- 10 bit split gamma (1024 coeff (512 + 512))
- 12 bit interpolated gamma (coeff 513)
So here, we have used the no of coeff to define which type of gamma we 
want to apply. So in the core gamma function you will find 4 cases:
switch(no_coeff)
case 0: disable gamma;
case 256: enable legacy gamma;
case 512: enable 10 bit split gamma;
case 513: enable 12 bit interpolated gamma;

This is the simplest implementation, and there is no need for any 
additional variable.
>
> Also, you need to make sure that the property size is aligned and reject
> the gamma table property if that's not the case, i.e.
>
>   if (blob_size % sizeof(drm_r32g32b32))
>   return -EINVAL;
>
>
> Plus then driver-specific code to reject anything that's not one of the
> supported sizes too.
>
> Of course that also needs igt test coverage.
> -Daniel
>
Shashank

WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()

[Borislav Petkov]

On Wed, Sep 23, 2015 at 06:06:21PM +0200, Borislav Petkov wrote:
> On Wed, Sep 23, 2015 at 04:44:50PM +0200, Daniel Vetter wrote:
> > sorry I sprinkled the locking stuff in the wrong places. Still confused
> > why the resume side doesn't blow up anywhere
> 
> But it does:
> 
> [   69.394204] BUG: unable to handle kernel NULL pointer dereference at 
> 0034
> [   69.402080] IP: [] pci_restore_msi_state+0x196/0x240
> [   69.408624] PGD 4162b8067 PUD 416581067 PMD 0 
> [   69.413122] Oops:  [#1] PREEMPT SMP 
> [   69.417101] Modules linked in: tun sha256_ssse3 sha256_generic drbg 
> binfmt_misc ipv6 vfat fat fuse dm_crypt dm_mod kv
> m_amd kvm crc32_pclmul aesni_intel aes_x86_64 lrw gf128mul glue_helper 
> ablk_helper cryptd amd64_edac_mod edac_mce_amd fa
> m15h_power k10temp amdkfd amd_iommu_v2 radeon acpi_cpufreq
> [   69.443647] CPU: 4 PID: 814 Comm: kworker/u16:5 Not tainted 4.3.0-rc2+ #3
> [   69.450430] Hardware name: To be filled by O.E.M. To be filled by 
> O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
> [   69.460336] Workqueue: events_unbound async_run_entry_fn
> [   69.465667] task: 88042a255f00 ti: 880428a68000 task.ti: 
> 880428a68000
> [   69.473145] RIP: 0010:[]  [] 
> pci_restore_msi_state+0x196/0x240
> [   69.482131] RSP: 0018:880428a6bc28  EFLAGS: 00010286
> [   69.487436] RAX:  RBX: 88042a308000 RCX: 
> 
> [   69.494568] RDX: 0001 RSI: 81304448 RDI: 
> 816c7a1b
> [   69.501700] RBP: 880428a6bc40 R08: 0001 R09: 
> 00522000
> [   69.508833] R10:  R11:  R12: 
> 
> [   69.515965] R13: 88042a3087b0 R14: 88042a308010 R15: 
> 88042a308038
> [   69.523097] FS:  7fc91328a700() GS:88042ce0() 
> knlGS:
> [   69.531185] CS:  0010 DS:  ES:  CR0: 8005003b
> [   69.536931] CR2: 0034 CR3: 0004164c7000 CR4: 
> 000406e0
> [   69.544061] Stack:
> [   69.546073]  0080002c2a3087b0  88042a308000 
> 880428a6bc78
> [   69.553525]  8130c141 88042a308098 88042a308000 
> 
> [   69.560996]  8804284e77a8 81961ef1 880428a6bc88 
> 8130c2b8
> [   69.568450] Call Trace:
> [   69.571044]  [] pci_restore_state.part.18+0xf1/0x250
> [   69.577706]  [] pci_restore_state+0x18/0x20
> [   69.583591]  [] pci_pm_restore_noirq+0x4c/0xd0
> [   69.589734]  [] ? pci_pm_freeze_noirq+0xf0/0xf0
> [   69.595966]  [] dpm_run_callback+0x77/0x2a0
> [   69.601850]  [] device_resume_noirq+0x93/0x150
> [   69.607994]  [] async_resume_noirq+0x1d/0x50
> [   69.613967]  [] async_run_entry_fn+0x46/0xf0
> [   69.619939]  [] process_one_work+0x1f8/0x640
> [   69.625910]  [] ? process_one_work+0x154/0x640
> [   69.632054]  [] worker_thread+0x4b/0x440
> [   69.637677]  [] ? process_one_work+0x640/0x640
> [   69.643822]  [] kthread+0xf6/0x110
> [   69.648927]  [] ? kthread_create_on_node+0x1f0/0x1f0
> [   69.655591]  [] ret_from_fork+0x3f/0x70
> [   69.661128]  [] ? kthread_create_on_node+0x1f0/0x1f0
> [   69.667794] Code: 66 89 4d ee 0f b7 c9 e8 79 41 fe ff 48 89 df e8 d1 7a ce 
> ff 0f b6 53 4b 8b 73 38 48 8d 4d ee 48 8b 7b 10 83 c2 02 e8 1a 31 fe ff <41> 
> 0f b6 4c 24 34 41 8b 54 24 30 be ff ff ff ff c0 e9 04 83 e1 
> [   69.687986] RIP  [] pci_restore_msi_state+0x196/0x240
> [   69.694772]  RSP 
> [   69.698412] CR2: 0034
> [   69.701879] ---[ end trace 814dd8cc56e427ae ]---

Ok, after some quick staring, we're at __pci_restore_msi_state():

pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, );
msi_mask_irq(entry, msi_mask(entry->msi_attrib.multi_cap),
 entry->masked);

which is:

.loc 1 411 0
movq%rbx, %rdi  # dev,
callarch_restore_msi_irqs   #
.LBB1921:
.LBB1922:
.loc 2 902 0
movzbl  75(%rbx), %edx  # dev_2(D)->msi_cap, D.31945
movl56(%rbx), %esi  # MEM[(const struct pci_dev *)dev_2(D)].devfn, 
MEM[(const struct pci_dev *)dev_2(D)].devfn
leaq-18(%rbp), %rcx #, tmp266
movq16(%rbx), %rdi  # MEM[(const struct pci_dev *)dev_2(D)].bus, 
MEM[(const struct pci_dev *)dev_2(D)].bus
addl$2, %edx#, D.31945
callpci_bus_read_config_word#
.LBE1922:
.LBE1921:
.loc 1 414 0
movzbl  52(%r12), %ecx  # *_85, tmp208  
<--- faulting insn
movl48(%r12), %edx  # _85->D.27233.D.27231.masked, D.31946
.LBB1923:
.LBB1924:
.loc 1 176 0
movl$-1, %esi   #, D.31951

and that %r12 is supposed to contain struct msi_desc *entry in
__pci_restore_msi_state():

entry = irq_get_msi_desc(dev->irq);

which is

.loc 4 654 0
movl1340(%rdi), %edi# dev_2(D)->irq, dev_2(D)->irq
callirq_get_irq_data#
.loc 4 655 0
testq   %rax, %rax  

WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()

[Borislav Petkov]

On Wed, Sep 23, 2015 at 04:44:50PM +0200, Daniel Vetter wrote:
> sorry I sprinkled the locking stuff in the wrong places. Still confused
> why the resume side doesn't blow up anywhere

But it does:

[   69.394204] BUG: unable to handle kernel NULL pointer dereference at 
0034
[   69.402080] IP: [] pci_restore_msi_state+0x196/0x240
[   69.408624] PGD 4162b8067 PUD 416581067 PMD 0 
[   69.413122] Oops:  [#1] PREEMPT SMP 
[   69.417101] Modules linked in: tun sha256_ssse3 sha256_generic drbg 
binfmt_misc ipv6 vfat fat fuse dm_crypt dm_mod kv
m_amd kvm crc32_pclmul aesni_intel aes_x86_64 lrw gf128mul glue_helper 
ablk_helper cryptd amd64_edac_mod edac_mce_amd fa
m15h_power k10temp amdkfd amd_iommu_v2 radeon acpi_cpufreq
[   69.443647] CPU: 4 PID: 814 Comm: kworker/u16:5 Not tainted 4.3.0-rc2+ #3
[   69.450430] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
[   69.460336] Workqueue: events_unbound async_run_entry_fn
[   69.465667] task: 88042a255f00 ti: 880428a68000 task.ti: 
880428a68000
[   69.473145] RIP: 0010:[]  [] 
pci_restore_msi_state+0x196/0x240
[   69.482131] RSP: 0018:880428a6bc28  EFLAGS: 00010286
[   69.487436] RAX:  RBX: 88042a308000 RCX: 
[   69.494568] RDX: 0001 RSI: 81304448 RDI: 816c7a1b
[   69.501700] RBP: 880428a6bc40 R08: 0001 R09: 00522000
[   69.508833] R10:  R11:  R12: 
[   69.515965] R13: 88042a3087b0 R14: 88042a308010 R15: 88042a308038
[   69.523097] FS:  7fc91328a700() GS:88042ce0() 
knlGS:
[   69.531185] CS:  0010 DS:  ES:  CR0: 8005003b
[   69.536931] CR2: 0034 CR3: 0004164c7000 CR4: 000406e0
[   69.544061] Stack:
[   69.546073]  0080002c2a3087b0  88042a308000 
880428a6bc78
[   69.553525]  8130c141 88042a308098 88042a308000 

[   69.560996]  8804284e77a8 81961ef1 880428a6bc88 
8130c2b8
[   69.568450] Call Trace:
[   69.571044]  [] pci_restore_state.part.18+0xf1/0x250
[   69.577706]  [] pci_restore_state+0x18/0x20
[   69.583591]  [] pci_pm_restore_noirq+0x4c/0xd0
[   69.589734]  [] ? pci_pm_freeze_noirq+0xf0/0xf0
[   69.595966]  [] dpm_run_callback+0x77/0x2a0
[   69.601850]  [] device_resume_noirq+0x93/0x150
[   69.607994]  [] async_resume_noirq+0x1d/0x50
[   69.613967]  [] async_run_entry_fn+0x46/0xf0
[   69.619939]  [] process_one_work+0x1f8/0x640
[   69.625910]  [] ? process_one_work+0x154/0x640
[   69.632054]  [] worker_thread+0x4b/0x440
[   69.637677]  [] ? process_one_work+0x640/0x640
[   69.643822]  [] kthread+0xf6/0x110
[   69.648927]  [] ? kthread_create_on_node+0x1f0/0x1f0
[   69.655591]  [] ret_from_fork+0x3f/0x70
[   69.661128]  [] ? kthread_create_on_node+0x1f0/0x1f0
[   69.667794] Code: 66 89 4d ee 0f b7 c9 e8 79 41 fe ff 48 89 df e8 d1 7a ce 
ff 0f b6 53 4b 8b 73 38 48 8d 4d ee 48 8b 7b 10 83 c2 02 e8 1a 31 fe ff <41> 0f 
b6 4c 24 34 41 8b 54 24 30 be ff ff ff ff c0 e9 04 83 e1 
[   69.687986] RIP  [] pci_restore_msi_state+0x196/0x240
[   69.694772]  RSP 
[   69.698412] CR2: 0034
[   69.701879] ---[ end trace 814dd8cc56e427ae ]---

This happens at resume - I caught the output over serial - screen is
dead, it doesn't show anything because it simply locks up/panics.

> ... Oh well. New patch below.

Yep, this one took care of the warning in
drm_helper_choose_encoder_dpms(). Thanks!

Now I need to go decypher that NULL ptr deref above.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

[PATCH 0/4] some optimization for evergreen cs

[Alex Deucher]

On Wed, Sep 23, 2015 at 2:59 AM, Dave Airlie  wrote:
> On 23 September 2015 at 09:03, Grazvydas Ignotas  wrote:
>> On Sun, Aug 23, 2015 at 3:57 AM, Grazvydas Ignotas  
>> wrote:
>>> These patches try to reduce CPU usage of register command checker
>>> without affecting functionality.
>>> For me this gives 3-4% perf improvement in glxgears and ~1% CPU usage 
>>> reduction
>>> in "The Talos Principle" CS thread.
>>>
>>> Grazvydas Ignotas (4):
>>>   drm/radeon: simplify register checker
>>>   drm/radeon: split evergreen_cs_check_reg
>>>   drm/radeon: refactor register check loop
>>>   drm/radeon: remove use of volatile qualifier
>>>
>>>  drivers/gpu/drm/radeon/evergreen_cs.c | 104 
>>> +++---
>>>  1 file changed, 47 insertions(+), 57 deletions(-)
>>
>> Can someone take a look at these? They still apply on current mainline
>> and I've been using them for a while without issues. I've also ran
>> piglit gpu tests and found no regressions.
>
> Reviewed-by: Dave Airlie 
>
> Alex can you pick these up?

Applied.  thanks!

Alex


>
> Dave.
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/15] drm/amdgpu: Spell vga_switcheroo consistently

[Alex Deucher]

On Sat, Sep 5, 2015 at 5:17 AM, Lukas Wunner  wrote:
> Currently everyone and their dog has their own favourite spelling
> for vga_switcheroo. This makes it hard to grep dmesg for log entries
> relating to vga_switcheroo. It also makes it hard to find related
> source files in the tree.
>
> vga_switcheroo.c uses pr_fmt "vga_switcheroo". Use that everywhere.
>
> Signed-off-by: Lukas Wunner 

Applied patches 13 and 14.

Thanks,

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> index 3f7aaa4..1a6b239 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
> @@ -536,7 +536,7 @@ static bool amdgpu_atpx_detect(void)
>
> if (has_atpx && vga_count == 2) {
> acpi_get_name(amdgpu_atpx_priv.atpx.handle, 
> ACPI_FULL_PATHNAME, );
> -   printk(KERN_INFO "VGA switcheroo: detected switching method 
> %s handle\n",
> +   printk(KERN_INFO "vga_switcheroo: detected switching method 
> %s handle\n",
>acpi_method_name);
> amdgpu_atpx_priv.atpx_detected = true;
> return true;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 6ff6ae9..b1a08d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1021,7 +1021,7 @@ static void amdgpu_check_arguments(struct amdgpu_device 
> *adev)
>   * amdgpu_switcheroo_set_state - set switcheroo state
>   *
>   * @pdev: pci dev pointer
> - * @state: vga switcheroo state
> + * @state: vga_switcheroo state
>   *
>   * Callback for the switcheroo driver.  Suspends or resumes the
>   * the asics before or after it is powered up using ACPI methods.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 2236793..a60eb4b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -488,7 +488,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
> *data, struct drm_file
>   *
>   * @dev: drm dev pointer
>   *
> - * Switch vga switcheroo state after last close (all asics).
> + * Switch vga_switcheroo state after last close (all asics).
>   */
>  void amdgpu_driver_lastclose_kms(struct drm_device *dev)
>  {
> --
> 1.8.5.2 (Apple Git-48)
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[pull] radeon and amdgpu drm-fixes-4.3

[Alex Deucher]

Hi Dave,

radeon and amdgpu fixes for 4.3.  It's a bit bigger than usual since
it's 3 weeks worth of fixes since I was on vacation, then at XDC.
- lots of stability fixes
- suspend and resume fixes
- GPU scheduler fixes
- Misc other fixes

The following changes since commit 9fbcc7c007ea200357e2453c6b2b153646fbc165:

  Merge branch 'linux-4.3' of 
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next (2015-09-11 
14:38:36 +1000)

are available in the git repository at:


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

for you to fetch changes up to e78654799135a788a941bacad3452fbd7083e518:

  drm/radeon: add quirk for MSI R7 370 (2015-09-23 17:23:47 -0400)


Alex Deucher (2):
  drm/amdgpu: Fix max_vblank_count value for current display engines
  drm/amdgpu: Sprinkle drm_modeset_lock_all to appease locking checks

Anatoli Antonovitch (1):
  drm/amdgpu: execution barrier after fence v2

Andrzej Hajda (1):
  drm/amdgpu: use kmemdup rather than duplicating its implementation

Christian König (13):
  drm/amdgpu: add option to disable semaphores
  drm/amdgpu: use write confirm for vm_flush()
  drm/amdgpu: signal scheduler fence when hw submission fails v3
  drm/amdgpu: move scheduler fence callback into fence v2
  drm/amdgpu: remove process_job callback from the scheduler
  drm/amdgpu: fix overflow on 32bit systems
  drm/amdgpu: export reservation_object from dmabuf to ttm (v2)
  drm/amdgpu: validate duplicates in the CS as well
  drm/amdgpu: use only one reservation object for each VM v2
  drm/amdgpu: cleanup entity init
  drm/amdgpu: rename fence->scheduler to sched v2
  drm/amdgpu: cleanup fence queue init v2
  drm/amdgpu: more scheduler cleanups v2

Chunming Zhou (1):
  drm/amdgpu: add tracepoint for scheduler (v2)

Dan Carpenter (4):
  drm/amdgpu: unwind properly in amdgpu_cs_parser_init()
  drm/amdgpu: integer overflow in amdgpu_info_ioctl()
  drm/amdgpu: info leak in amdgpu_gem_metadata_ioctl()
  drm/amdgpu: integer overflow in amdgpu_mode_dumb_create()

Daniel Vetter (1):
  drm/radeon: Sprinkle drm_modeset_lock_all to appease locking checks

Junwei Zhang (2):
  drm/amdgpu: refine the job naming for amdgpu_job and amdgpu_sched_job
  drm/amdgpu: refine the scheduler job type conversion

Leo Liu (4):
  drm/amdgpu: Disable UVD PG
  drm/amdgpu: make UVD handle checking more strict
  drm/amdgpu: fix the UVD suspend sequence order
  drm/amdgpu: fix UVD suspend and resume for VI APU

Maxim Sheviakov (1):
  drm/radeon: add quirk for MSI R7 370

monk.liu (1):
  drm/amdgpu: sync ce and me with SWITCH_BUFFER(2)

 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  11 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c  |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c   |   6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c |   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 137 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c |  10 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |   9 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |   4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c   |  25 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c|   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  16 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c  |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  19 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c   |   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c|   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c  |  47 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c   |  65 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c|  27 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c|   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c   |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  80 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  45 ++-
 drivers/gpu/drm/amd/amdgpu/cz_smc.c |   6 +-
 drivers/gpu/drm/amd/amdgpu/fiji_smc.c   |   4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  74 ---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   |  79 +---
 drivers/gpu/drm/amd/amdgpu/iceland_smc.c|   2 +-
 drivers/gpu/drm/amd/amdgpu/tonga_smc.c  |   4 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c   |   4 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c   |   4 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   |  20 +--
 drivers/gpu/drm/amd/amdgpu/vi.c |   3 +-
 

[PATCH 0/4] some optimization for evergreen cs

[Dave Airlie]

On 23 September 2015 at 09:03, Grazvydas Ignotas  wrote:
> On Sun, Aug 23, 2015 at 3:57 AM, Grazvydas Ignotas  
> wrote:
>> These patches try to reduce CPU usage of register command checker
>> without affecting functionality.
>> For me this gives 3-4% perf improvement in glxgears and ~1% CPU usage 
>> reduction
>> in "The Talos Principle" CS thread.
>>
>> Grazvydas Ignotas (4):
>>   drm/radeon: simplify register checker
>>   drm/radeon: split evergreen_cs_check_reg
>>   drm/radeon: refactor register check loop
>>   drm/radeon: remove use of volatile qualifier
>>
>>  drivers/gpu/drm/radeon/evergreen_cs.c | 104 
>> +++---
>>  1 file changed, 47 insertions(+), 57 deletions(-)
>
> Can someone take a look at these? They still apply on current mainline
> and I've been using them for a while without issues. I've also ran
> piglit gpu tests and found no regressions.

Reviewed-by: Dave Airlie 

Alex can you pick these up?

Dave.

WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()

[Daniel Vetter]

On Wed, Sep 23, 2015 at 10:59:51AM +0200, Borislav Petkov wrote:
> On Wed, Sep 23, 2015 at 09:25:23AM +0200, Daniel Vetter wrote:
> > Strange thing is that I've tested this on a radeon over here and I don't
> > see this backtrace ... wut. Below diff should appease the backtraces at
> > least.
> 
> Doesn't look like it.

sorry I sprinkled the locking stuff in the wrong places. Still confused
why the resume side doesn't blow up anywhere ... Oh well. New patch below.

Thanks, Daniel

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index d8319dae8358..f3f562f6d848 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1573,10 +1573,12 @@ int radeon_suspend_kms(struct drm_device *dev, bool 
suspend, bool fbcon)

drm_kms_helper_poll_disable(dev);

+   drm_modeset_lock_all(dev);
/* turn off display hw */
list_for_each_entry(connector, >mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
}
+   drm_modeset_unlock_all(dev);

/* unpin the front buffers and cursors */
list_for_each_entry(crtc, >mode_config.crtc_list, head) {
@@ -1734,9 +1736,11 @@ int radeon_resume_kms(struct drm_device *dev, bool 
resume, bool fbcon)
if (fbcon) {
drm_helper_resume_force_mode(dev);
/* turn on display hw */
+   drm_modeset_lock_all(dev);
list_for_each_entry(connector, 
>mode_config.connector_list, head) {
drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON);
}
+   drm_modeset_unlock_all(dev);
}

drm_kms_helper_poll_enable(dev);
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[patch 1/4] drm/amdgpu: unwind properly in amdgpu_cs_parser_init()

[Christian König]

On 23.09.2015 12:59, Dan Carpenter wrote:
> The amdgpu_cs_parser_init() function doesn't clean up after itself but
> instead the caller uses a free everything function amdgpu_cs_parser_fini()
> on failure.  This style of error handling is often buggy.  In this
> example, we call "drm_free_large(parser->chunks[i].kdata);" when it is
> an unintialized pointer or when "parser->chunks" is NULL.
>
> I fixed this bug by adding unwind code so that it frees everything that
> it allocates.
>
> I also mode some other very minor changes:
> 1) Renamed "r" to "ret".
> 2) Moved the chunk_array allocation to the start of the function.
> 3) Removed some initializers which are no longer needed.
>
> Reported-by: Ilja Van Sprundel 
> Signed-off-by: Dan Carpenter 

The whole set looks sane to me, patches are Reviewed-by: Christian König 


Regards,
Christian.

>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 3b355ae..abb257d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -154,42 +154,41 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
> void *data)
>   {
>   union drm_amdgpu_cs *cs = data;
>   uint64_t *chunk_array_user;
> - uint64_t *chunk_array = NULL;
> + uint64_t *chunk_array;
>   struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>   unsigned size, i;
> - int r = 0;
> + int ret;
>   
> - if (!cs->in.num_chunks)
> - goto out;
> + if (cs->in.num_chunks == 0)
> + return 0;
> +
> + chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t), 
> GFP_KERNEL);
> + if (!chunk_array)
> + return -ENOMEM;
>   
>   p->ctx = amdgpu_ctx_get(fpriv, cs->in.ctx_id);
>   if (!p->ctx) {
> - r = -EINVAL;
> - goto out;
> + ret = -EINVAL;
> + goto free_chunk;
>   }
> +
>   p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
>   
>   /* get chunks */
>   INIT_LIST_HEAD(>validated);
> - chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t), 
> GFP_KERNEL);
> - if (chunk_array == NULL) {
> - r = -ENOMEM;
> - goto out;
> - }
> -
>   chunk_array_user = (uint64_t __user *)(cs->in.chunks);
>   if (copy_from_user(chunk_array, chunk_array_user,
>  sizeof(uint64_t)*cs->in.num_chunks)) {
> - r = -EFAULT;
> - goto out;
> + ret = -EFAULT;
> + goto put_bo_list;
>   }
>   
>   p->nchunks = cs->in.num_chunks;
>   p->chunks = kmalloc_array(p->nchunks, sizeof(struct amdgpu_cs_chunk),
>   GFP_KERNEL);
> - if (p->chunks == NULL) {
> - r = -ENOMEM;
> - goto out;
> + if (!p->chunks) {
> + ret = -ENOMEM;
> + goto put_bo_list;
>   }
>   
>   for (i = 0; i < p->nchunks; i++) {
> @@ -200,8 +199,9 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
> void *data)
>   chunk_ptr = (void __user *)chunk_array[i];
>   if (copy_from_user(_chunk, chunk_ptr,
>  sizeof(struct drm_amdgpu_cs_chunk))) {
> - r = -EFAULT;
> - goto out;
> + ret = -EFAULT;
> + i--;
> + goto free_partial_kdata;
>   }
>   p->chunks[i].chunk_id = user_chunk.chunk_id;
>   p->chunks[i].length_dw = user_chunk.length_dw;
> @@ -212,13 +212,14 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
> void *data)
>   
>   p->chunks[i].kdata = drm_malloc_ab(size, sizeof(uint32_t));
>   if (p->chunks[i].kdata == NULL) {
> - r = -ENOMEM;
> - goto out;
> + ret = -ENOMEM;
> + i--;
> + goto free_partial_kdata;
>   }
>   size *= sizeof(uint32_t);
>   if (copy_from_user(p->chunks[i].kdata, cdata, size)) {
> - r = -EFAULT;
> - goto out;
> + ret = -EFAULT;
> + goto free_partial_kdata;
>   }
>   
>   switch (p->chunks[i].chunk_id) {
> @@ -238,15 +239,15 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
> void *data)
>   gobj = drm_gem_object_lookup(p->adev->ddev,
>p->filp, handle);
>   if (gobj == NULL) {
> - r = -EINVAL;
> - goto out;
> + ret = -EINVAL;
> + goto free_partial_kdata;
>   }
>   
>   p->uf.bo = 

[Bug 102401] Radeon Displayport Audio Warping

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

https://bugzilla.kernel.org/show_bug.cgi?id=102401

--- Comment #12 from Alex Deucher  ---
I don't see what this patch changes.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 04/23] drm: Add drm structures for palette color property

[Daniel Vetter]

On Wed, Sep 23, 2015 at 06:29:31PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 9/23/2015 6:19 PM, Daniel Vetter wrote:
> >On Wed, Sep 23, 2015 at 01:45:16PM +0530, Sharma, Shashank wrote:
> >>Regards
> >>Shashank
> >>
> >>On 9/22/2015 6:38 PM, Daniel Vetter wrote:
> >>>On Wed, Sep 16, 2015 at 11:07:01PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi 
> 
> This patch adds new structures in DRM layer for Palette color
> correction.These structures will be used by user space agents
> to configure appropriate number of samples and Palette LUT for
> a platform.
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 
> ---
>   include/uapi/drm/drm.h | 27 +++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index e3c642f..f72b916 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -840,6 +840,33 @@ struct drm_palette_caps {
>   __u32 num_samples_after_ctm;
>   };
> 
> +struct drm_r32g32b32 {
> + /*
> +  * Data is in U8.24 fixed point format.
> +  * All platforms support values within [0, 1.0] range,
> +  * for Red, Green and Blue colors.
> +  */
> + __u32 r32;
> + __u32 g32;
> + __u32 b32;
> >>>
> >>>It's not strictly required, but adding a __u32 reserved here to align the
> >>>struct to 64 bits seems good imo. Slight overhead but meh about that.
> >>Humm, ok, we can check this out.
> >>>
> +};
> +
> +struct drm_palette {
> + /* Structure version. Should be 1 currently */
> + __u32 version;
> >>>
> >>>Definitely great practice to take compat into account and definitely
> >>>needed for the first design using ioctls but I don't think we need this
> >>>here. Properties are already extinsible themselves: We can just greate a
> >>>"ctm-v2", "ctm-v3" if the layout changes, and since the actual ctm matrix
> >>>is stored in the drm_crtc_state any compat code on the kernel will be
> >>>shared.
> >>>
> >>>Aside: For an ioctl the recommended way to handle backwards compat and
> >>>extensions in drm is with a flags bitfield. That's more flexible than a
> >>>linear version field, and extending the ioctl struct at the end is already
> >>>handled by the drm core in a transparent fashion (it 0-fills either kernel
> >>>or userspace side).
> >>>
> >>Agree, we will drop this. Do you think we should add a flags field, or is it
> >>ok without it ?
> >
> >No need for a flag field since this is not an ioctl struct. That "Aside:"
> >was really meant as a comment aside and not relevant for properties.
> >
> + /*
> +  * This has to be a supported value during get call.
> +  * Feature will be disabled if this is 0 while set
> +  */
> + __u32 num_samples;
> >>>
> >>>blob properties already have a size, storing it again in the blob is
> >>>redundnant. Instead I think a small helper to get the number of samples
> >>>for a given gamma table blob would be needed.
> >>>
> >>>Cheers, Daniel
> >>Please note that they are different. One is the size of blob and other one
> >>is the num_samples supported by the property, in the current correction
> >>mode. If you check the design doc, num_sample serves the purpose of deciding
> >>which correction mode to be applied also. fox ex, for gamma, num_samples=0
> >>indicates disable gamma, whereas num_samples=512 indicates split gamma mode.
> >
> >num_samples = blob_size/(sizeof(drm_r32g32b32));
> >
> >I just think that this information is redundant and if userspace supplies
> >a gamma table with the wrong size we should just reject it. There's really
> >no reason for userspace to create a blob property where the size doesn't
> >exactly match the gamma table.
> >
> >I guess again that this was needed for the ioctl where there's no sideband
> >for the size. But properties _are_ sized.
> Again, this is what we decided in the design discussion. The driver will
> showcase the best option for property, but that doesn't stop a user space
> with more knowledge of HW to send other supported options. for example, in
> case of gamma, the driver supports all 3 possible modes:
> - 8 bit legacy gamma (256 coeff)
> - 10 bit split gamma (1024 coeff (512 + 512))
> - 12 bit interpolated gamma (coeff 513)
> So here, we have used the no of coeff to define which type of gamma we want
> to apply. So in the core gamma function you will find 4 cases:
> switch(no_coeff)
> case 0: disable gamma;
> case 256: enable legacy gamma;
> case 512: enable 10 bit split gamma;
> case 513: enable 12 bit interpolated gamma;
> 
> This is the simplest implementation, and there is no need for any additional
> variable.

I'm confused, since this is exactly what I'm suggesting. My only
observation is that we don't need a separate num_samples field in the blob
structure itself since userspace already needs to tell the kernel the size
of the blob 

[PATCH 02/23] drm: Add structure for querying palette color capabilities

[Daniel Vetter]

On Wed, Sep 23, 2015 at 11:57:58AM +, Sharma, Shashank wrote:
> This would be an interface/design change, to change from one blob of
> correction property, to split into multiple query properties for
> palette_before_blob and palette_after_blob.  Please let me know if this
> is really required ?

Yes I think splitting this up into one property per value is the right
approach. That's also why we split the before/after gamma tables and the
ctm each into it's own property.

And I don't think this is a design change of the interface - we still
exchange the exact same values with the exact semantics between kernel and
userspace, there's just a small difference in the actual transport
protocol. Those kinds of minor changes happen all the time when
upstreaming features. And that's the reason why we absolutely _must_ have
a close collaboration between the kernel and userspace side. Which
unfortunately on this feature here took a few months to get going, but
hopefully with shared git trees and talking on irc now that should be
easier.
-Daniel

> 
> Regards
> Shashank
> -Original Message-
> From: Smith, Gary K 
> Sent: Wednesday, September 23, 2015 3:17 PM
> To: Sharma, Shashank; Daniel Vetter; Roper, Matthew D
> Cc: Matheson, Annie J; Bradford, Robert; Palleti, Avinash Reddy; intel-gfx at 
> lists.freedesktop.org; dri-devel at lists.freedesktop.org; Mukherjee, 
> Indranil; Bish, Jim; kausalmalladi at gmail.com; Vetter, Daniel
> Subject: RE: [PATCH 02/23] drm: Add structure for querying palette color 
> capabilities
> 
> Given that its only one word of info per LUT, I'm OK with it being two 
> separate properties. 
> I believe it was much more complex previously with a lot more info per LUT, 
> which is probably why I preferred a blob.
> 
> Thanks
> Gary
> 
> -Original Message-
> From: Sharma, Shashank
> Sent: Wednesday, September 23, 2015 9:10 AM
> To: Daniel Vetter; Roper, Matthew D
> Cc: Matheson, Annie J; Bradford, Robert; Palleti, Avinash Reddy; intel-gfx at 
> lists.freedesktop.org; dri-devel at lists.freedesktop.org; Mukherjee, 
> Indranil; Bish, Jim; Smith, Gary K; kausalmalladi at gmail.com; Vetter, Daniel
> Subject: Re: [PATCH 02/23] drm: Add structure for querying palette color 
> capabilities
> 
> Hi Matt, Daniel
> Addressing the review comments from both of you here.
> 
> Regards
> Shashank
> 
> On 9/22/2015 6:32 PM, Daniel Vetter wrote:
> > On Wed, Sep 16, 2015 at 10:51:31AM -0700, Matt Roper wrote:
> >> On Wed, Sep 16, 2015 at 11:06:59PM +0530, Shashank Sharma wrote:
> >>> From: Kausal Malladi 
> >>>
> >>> The DRM color management framework is targeting various hardware 
> >>> platforms and drivers. Different platforms can have different color 
> >>> correction and enhancement capabilities.
> >>>
> >>> A commom user space application can query these capabilities using 
> >>> the DRM property interface. Each driver can fill this property with 
> >>> its platform's palette color capabilities.
> >>>
> >>> This patch adds new structure in DRM layer for querying palette 
> >>> color capabilities. This structure will be used by all user space 
> >>> agents to configure appropriate color configurations.
> >>>
> >>> Signed-off-by: Shashank Sharma 
> >>> Signed-off-by: Kausal Malladi 
> >>
> >> I think you provided an explanation on a previous code review cycle, 
> >> but I forget the details now...what's the benefit to using a blob for 
> >> caps rather than having these be individual properties?  Individual 
> >> properties seems more natural to me, but I think you had a 
> >> justification for blobbing them together; that reasoning would be 
> >> good to include in the commit message.
> >
> > Yeah I'm leaning slightly towards individual props too, that would 
> > give us a bit more freedom with placing them (e.g. if someone comes up 
> > with funky hw where before_ctm and ctm are per-plane and after_ctm is 
> > on the crtc, with only some planes support the before_ctm gamma table).
> This was the part where we spent most of the time during the design review, 
> and the reason we came up for this was:
> - This is a read only property, which userspace would like to read only once, 
> and cache the information. It was also Gary's opinion to keep this as single 
> blob for all.
> - Making individual property needs more information to be provided to user 
> space.
> - This is a blob only for pipe level capabilities, the plane level blob will 
> be separate from this.
> - We can handle this HW also, by loading proper plane and pipe level 
> capability blob. This is more convenient to have all the capabilities 
> together at the same place, than keep on querying the same.
> >
> > Also if you do per-prop properties instead of the blob you can drop 
> > the version/reserved fields, since properties are inheritedly designed 
> > to be extendible. So no need to revision them again (it only leads to 
> > more code that might break).
> > -Daniel
> >
> We are anyways planning to drop the version, as 

[Intel-gfx] [PATCH 10/23] drm/i915: Add gamma correction handlers

[Daniel Vetter]

On Wed, Sep 23, 2015 at 01:52:21PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 9/22/2015 6:45 PM, Daniel Vetter wrote:
> >On Wed, Sep 16, 2015 at 11:07:07PM +0530, Shashank Sharma wrote:
> >>I915 driver registers gamma correction as palette correction
> >>property with DRM layer. This patch adds set_property() and get_property()
> >>handlers for pipe level gamma correction.
> >>
> >>The set function attaches the Gamma correction blob to CRTC state, these
> >>values will be committed during atomic commit.
> >>
> >>Signed-off-by: Shashank Sharma 
> >>Signed-off-by: Kausal Malladi 
> >>---
> >>  drivers/gpu/drm/i915/intel_atomic.c| 20 
> >>  drivers/gpu/drm/i915/intel_color_manager.c | 21 +
> >>  drivers/gpu/drm/i915/intel_drv.h   |  5 +
> >>  3 files changed, 46 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> >>b/drivers/gpu/drm/i915/intel_atomic.c
> >>index 500d2998..0b61fef 100644
> >>--- a/drivers/gpu/drm/i915/intel_atomic.c
> >>+++ b/drivers/gpu/drm/i915/intel_atomic.c
> >>@@ -315,6 +315,13 @@ int 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 = >mode_config;
> >>+
> >>+   if (property == config->cm_palette_after_ctm_property)
> >>+   return intel_color_manager_set_pipe_gamma(dev, state,
> >>+   >base, val);
> >>+
> >>DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> >>return -EINVAL;
> >>  }
> >>@@ -324,6 +331,19 @@ int intel_crtc_atomic_get_property(struct drm_crtc 
> >>*crtc,
> >>   struct drm_property *property,
> >>   uint64_t *val)
> >>  {
> >>+   struct drm_device *dev = crtc->dev;
> >>+   struct drm_mode_config *config = >mode_config;
> >>+
> >>+   if (property == config->cm_palette_after_ctm_property) {
> >>+   *val = (state->palette_after_ctm_blob) ?
> >>+   state->palette_after_ctm_blob->base.id : 0;
> >>+   goto found;
> >>+   }
> >
> >Since color manager properties are meant as a new standardize KMS
> >extension (we put them into the core drm_crtc_state) the get/set support
> >should also be in the core. See e.g. how the rotation property is handled
> >in drm_atomic_plane_get/set_property. So all this code should be added to
> >drm_atomic_crtc_get/set_property.
> Thanks, sounds like a good one. Will move this.

When moving this please don't forget to add the blob->length sanity checks
mentioned in another thread in this discussion.

> >
> >
> >>+
> >>DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> >>return -EINVAL;
> >>+
> >>+found:
> >>+   DRM_DEBUG_KMS("Found property %s\n", property->name);
> >>+   return 0;
> >>  }
> >>diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> >>b/drivers/gpu/drm/i915/intel_color_manager.c
> >>index 77f58f2..9421bb6 100644
> >>--- a/drivers/gpu/drm/i915/intel_color_manager.c
> >>+++ b/drivers/gpu/drm/i915/intel_color_manager.c
> >>@@ -27,6 +27,27 @@
> >>
> >>  #include "intel_color_manager.h"
> >>
> >>+int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
> >>+   struct drm_crtc_state *crtc_state,
> >>+   struct drm_mode_object *obj, uint32_t blob_id)
> >>+{
> >>+   struct drm_property_blob *blob;
> >>+
> >>+   blob = drm_property_lookup_blob(dev, blob_id);
> >>+   if (!blob) {
> >>+   DRM_DEBUG_KMS("Invalid Blob ID\n");
> >>+   return -EINVAL;
> >>+   }
> >>+
> >>+   if (crtc_state->palette_after_ctm_blob)
> >>+   drm_property_unreference_blob(
> >>+   crtc_state->palette_after_ctm_blob);
> >>+
> >>+   /* Attach the blob to be committed in state */
> >>+   crtc_state->palette_after_ctm_blob = blob;
> >>+   return 0;
> >>+}
> >
> >What is this used for? It looks a bit like legacy property code, and we
> >have a generic helper to make that happen
> >(drm_atomic_helper_crtc_set_property).
> >
> No Daniel, its not the legacy part. Please check atomic_begin() part in the
> next patches. This is like the top half of atomic set_property for CRTC and
> will be applicable for any property. As you already know, to set a property:
> - userspace creates a blob, and sends the blob id via set_property()
> interface
> - set_property() saves the blob_id in the corresponding pointer space in the
> crtc_state()
> - From the atomic_commit_begin() we will extract the values from blob and
> start committing to the registers. Its kind of bottom_half() of atomic
> set_property for CRTC.
> >Generally please don't add functions/structs without also adding a user,
> >it means that reviewers have to constantly jump around in your patch
> >series to figure out how something is used. Instead if you want to split
> >things up really fine 

[Intel-gfx] [PATCH 09/23] drm/i915: Register pipe color capabilities

[Daniel Vetter]

On Wed, Sep 23, 2015 at 02:05:25PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 9/22/2015 6:54 PM, Daniel Vetter wrote:
> >On Wed, Sep 16, 2015 at 11:07:06PM +0530, Shashank Sharma wrote:
> >>DRM color manager contains these color properties:
> >>
> >>1. "crtc_palette_capabilities_property": to allow a
> >>core driver to load and showcase its color correction
> >>capabilities to user space.
> >>2. "ctm": Color transformation matrix property, where a
> >>color transformation matrix of 9 correction values gets
> >>applied as correction.
> >>3. "palette_before_ctm": for corrections which get applied
> >>beore color transformation matrix correction.
> >>4. "palette_after_ctm": for corrections which get applied
> >>after color transformation matrix correction.
> >>
> >>Intel color manager registers:
> >>1. Gamma correction property as "palette_after_ctm" property
> >>2. Degamma correction capability as "palette_bafore_ctm" property
> >>capability as "palette_after_ctm" DRM color property hook.
> >>3. CSC as "ctm" property.
> >>
> >>This patch does the following:
> >>1. Add a function which loads the platform's color correction
> >>capabilities in the cm_crtc_palette_capabilities_property structure.
> >>2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
> >>getting initiaized.
> >>3. Adds two new parameters "num_samples_after_ctm" and
> >>"num_samples_before_ctm" in intel_device_info as gamma and
> >>degamma coefficients vary per platform basis.
> >>
> >>Signed-off-by: Shashank Sharma 
> >>---
> >>  drivers/gpu/drm/i915/i915_drv.h|  2 ++
> >>  drivers/gpu/drm/i915/intel_color_manager.c | 45 
> >> ++
> >>  drivers/gpu/drm/i915/intel_color_manager.h |  2 ++
> >>  3 files changed, 49 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> >>b/drivers/gpu/drm/i915/i915_drv.h
> >>index 3bf8a9b..22de2cb 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -798,6 +798,8 @@ struct intel_device_info {
> >>u8 num_sprites[I915_MAX_PIPES];
> >>u8 gen;
> >>u8 ring_mask; /* Rings supported by the HW */
> >>+   u16 num_samples_after_ctm;
> >>+   u16 num_samples_before_ctm;
> >>DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
> >>/* Register offsets for the various display pipes and transcoders */
> >>int pipe_offsets[I915_MAX_TRANSCODERS];
> >>diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> >>b/drivers/gpu/drm/i915/intel_color_manager.c
> >>index 7357d99..77f58f2 100644
> >>--- a/drivers/gpu/drm/i915/intel_color_manager.c
> >>+++ b/drivers/gpu/drm/i915/intel_color_manager.c
> >>@@ -27,7 +27,52 @@
> >>
> >>  #include "intel_color_manager.h"
> >>
> >>+int get_pipe_capabilities(struct drm_device *dev,
> >>+   struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
> >>+{
> >>+   struct drm_property_blob *blob;
> >>+
> >>+   /*
> >>+* This function loads best capability for gamma correction
> >>+* For example:
> >>+* CHV best Gamma correction (CGM unit, 10 bit)
> >>+* has 257 entries, best degamma is 65 entries
> >>+*/
> >>+   palette_caps->version = COLOR_STRUCT_VERSION;
> >>+   palette_caps->num_samples_after_ctm =
> >>+   INTEL_INFO(dev)->num_samples_after_ctm;
> >>+   palette_caps->num_samples_before_ctm =
> >>+   INTEL_INFO(dev)->num_samples_before_ctm;
> >>+   blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
> >>+   (const void *) palette_caps);
> >>+   if (IS_ERR_OR_NULL(blob)) {
> >>+   DRM_ERROR("Create blob for capabilities failed\n");
> >>+   return PTR_ERR(blob);
> >>+   }
> >>+
> >>+   return blob->base.id;
> >>+}
> >>+
> >>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> >>struct drm_mode_object *mode_obj)
> >>  {
> >>+   struct drm_mode_config *config = >mode_config;
> >>+   struct drm_palette_caps *palette_caps;
> >>+   struct drm_crtc *crtc;
> >>+   int capabilities_blob_id;
> >>+
> >>+   crtc = obj_to_crtc(mode_obj);
> >>+   palette_caps = kzalloc(sizeof(struct drm_palette_caps),
> >>+   GFP_KERNEL);
> >>+   capabilities_blob_id = get_pipe_capabilities(dev,
> >>+   palette_caps, crtc);
> >>+
> >>+   if (config->cm_crtc_palette_capabilities_property) {
> >>+   drm_object_attach_property(mode_obj,
> >>+   config->cm_crtc_palette_capabilities_property,
> >>+   capabilities_blob_id);
> >
> >I didn't find any code to attach the before/after_ctm gamma properties and
> >the ctm property. Also I think a small helper would be really nice here in
> >the drm core:
> >
> Patch 15,16,17 adds those for gamma, degamma and CSC. This is just for the
> capabilities property.

Indeed, my mail search failed me.

> >drm_crtc_attach_cc_properties(struct drm_crtc *crtc,
> >   int 

[PATCH 04/23] drm: Add drm structures for palette color property

[Daniel Vetter]

On Wed, Sep 23, 2015 at 01:45:16PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 9/22/2015 6:38 PM, Daniel Vetter wrote:
> >On Wed, Sep 16, 2015 at 11:07:01PM +0530, Shashank Sharma wrote:
> >>From: Kausal Malladi 
> >>
> >>This patch adds new structures in DRM layer for Palette color
> >>correction.These structures will be used by user space agents
> >>to configure appropriate number of samples and Palette LUT for
> >>a platform.
> >>
> >>Signed-off-by: Shashank Sharma 
> >>Signed-off-by: Kausal Malladi 
> >>---
> >>  include/uapi/drm/drm.h | 27 +++
> >>  1 file changed, 27 insertions(+)
> >>
> >>diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> >>index e3c642f..f72b916 100644
> >>--- a/include/uapi/drm/drm.h
> >>+++ b/include/uapi/drm/drm.h
> >>@@ -840,6 +840,33 @@ struct drm_palette_caps {
> >>__u32 num_samples_after_ctm;
> >>  };
> >>
> >>+struct drm_r32g32b32 {
> >>+   /*
> >>+* Data is in U8.24 fixed point format.
> >>+* All platforms support values within [0, 1.0] range,
> >>+* for Red, Green and Blue colors.
> >>+*/
> >>+   __u32 r32;
> >>+   __u32 g32;
> >>+   __u32 b32;
> >
> >It's not strictly required, but adding a __u32 reserved here to align the
> >struct to 64 bits seems good imo. Slight overhead but meh about that.
> Humm, ok, we can check this out.
> >
> >>+};
> >>+
> >>+struct drm_palette {
> >>+   /* Structure version. Should be 1 currently */
> >>+   __u32 version;
> >
> >Definitely great practice to take compat into account and definitely
> >needed for the first design using ioctls but I don't think we need this
> >here. Properties are already extinsible themselves: We can just greate a
> >"ctm-v2", "ctm-v3" if the layout changes, and since the actual ctm matrix
> >is stored in the drm_crtc_state any compat code on the kernel will be
> >shared.
> >
> >Aside: For an ioctl the recommended way to handle backwards compat and
> >extensions in drm is with a flags bitfield. That's more flexible than a
> >linear version field, and extending the ioctl struct at the end is already
> >handled by the drm core in a transparent fashion (it 0-fills either kernel
> >or userspace side).
> >
> Agree, we will drop this. Do you think we should add a flags field, or is it
> ok without it ?

No need for a flag field since this is not an ioctl struct. That "Aside:"
was really meant as a comment aside and not relevant for properties.

> >>+   /*
> >>+* This has to be a supported value during get call.
> >>+* Feature will be disabled if this is 0 while set
> >>+*/
> >>+   __u32 num_samples;
> >
> >blob properties already have a size, storing it again in the blob is
> >redundnant. Instead I think a small helper to get the number of samples
> >for a given gamma table blob would be needed.
> >
> >Cheers, Daniel
> Please note that they are different. One is the size of blob and other one
> is the num_samples supported by the property, in the current correction
> mode. If you check the design doc, num_sample serves the purpose of deciding
> which correction mode to be applied also. fox ex, for gamma, num_samples=0
> indicates disable gamma, whereas num_samples=512 indicates split gamma mode.

num_samples = blob_size/(sizeof(drm_r32g32b32));

I just think that this information is redundant and if userspace supplies
a gamma table with the wrong size we should just reject it. There's really
no reason for userspace to create a blob property where the size doesn't
exactly match the gamma table.

I guess again that this was needed for the ioctl where there's no sideband
for the size. But properties _are_ sized.

Also, you need to make sure that the property size is aligned and reject
the gamma table property if that's not the case, i.e.

if (blob_size % sizeof(drm_r32g32b32))
return -EINVAL;


Plus then driver-specific code to reject anything that's not one of the
supported sizes too.

Of course that also needs igt test coverage.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Intel-gfx] [PATCH] drm: Use userspace compatible type in fourcc_mod_code macro

[Ville Syrjälä]

On Wed, Sep 23, 2015 at 10:10:31AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> __u64 should be used instead of u64.
> 
> Feature originally added in:
> 
> commit e3eb3250d84ef97b766312345774367b6a310db8
> Author: Rob Clark 
> Date:   Thu Feb 5 14:41:52 2015 +
> 
> drm: add support for tiled/compressed/etc modifier in addfb2
> 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Rob Clark 
> Cc: Daniel Stone 
> Cc: Daniel Vetter 
> Cc: dri-devel at lists.freedesktop.org
> Cc: stable at vger.kernel.org

Reviewed-by: Ville Syrjälä 

> ---
>  include/uapi/drm/drm_fourcc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 8c5e8b91a3cb..0b69a7753558 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -158,7 +158,7 @@
>  /* add more to the end as needed */
>  
>  #define fourcc_mod_code(vendor, val) \
> - u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
> 0x00ffULL))
> + __u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
> 0x00ffULL))
>  
>  /*
>   * Format Modifier tokens:
> -- 
> 2.5.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

--- Comment #5 from Balázs Vinarz  ---
I've updated the post.

-- 
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/20150923/fdfcb68c/attachment-0001.html>

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

--- Comment #4 from Balázs Vinarz  ---
Created attachment 118415
  --> https://bugs.freedesktop.org/attachment.cgi?id=118415=edit
xorg.0.log

-- 
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/20150923/e1b1c506/attachment.html>

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

--- Comment #3 from Balázs Vinarz  ---
Created attachment 118414
  --> https://bugs.freedesktop.org/attachment.cgi?id=118414=edit
dmesg

-- 
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/20150923/a5a90c60/attachment.html>

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

Balázs Vinarz  changed:

   What|Removed |Added

 Attachment #118413|0   |1
is obsolete||

-- 
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/20150923/96413d48/attachment.html>

[Intel-gfx] [PATCH 00/23] Color Management for DRM

[Sharma, Shashank]

Regards
Shashank

On 9/22/2015 6:57 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:06:57PM +0530, Shashank Sharma wrote:
>> This patch set adds Color Manager implementation in DRM layer. Color Manager
>> is an extension in DRM framework to support color correction/enhancement.
>>
>> Various Hardware platforms can support several color correction capabilities.
>> Color Manager provides abstraction of these capabilities and allows a user
>> space UI agent to correct/enhance the display using the DRM property 
>> interface.
>>
>> How is this going to work?
>> ==
>> 1. This patch series adds a few new properties in DRM framework. These 
>> properties are:
>>  a. color_capabilities property (type blob)
>>  b. Color Transformation Matrix property for corrections like CSC 
>> (called CTM, type blob)
>>  c. Palette correction properties for corrections like gamma fixup 
>> (called palette_correction, type blob)
>> 2. Also, this patch series adds few structures to indicate specifications of 
>> a property like size, no_of_samples for correction etc.
>> 3. These properties are present in mode_config.
>> 4. When the platform's display driver loads, it fills up the values of 
>> color_capabilities property using the standard structures (added in step 2).
>> For example, Intel's I915 driver adds following color correction 
>> capabilities:
>>  a. gamma correction capability as palette correction property, with 
>> 257 correction coefficients and a max/min value
>>  b. csc correction capability as CTM correction property, with 3x3 
>> transformation matrix values and max/min values
>> 5. Now when userspace comes up, it queries the platform's color capabilities 
>> by doing a get_property() on color_capabilities DRM property
>> 6. Reading the blob, the userspace understands the color capabilities of the 
>> platform.
>> For example, userspace will understand it can support:
>>  a. palette_correction with 257 coefficients
>>  b. CSC correction with 3x3 = 9 values
>> 7. To set color correction values, userspace:
>>  a. creates a blob using the create_blob_ioctl in standard 
>> palette_correction structure format, with the correction values
>>  b. calls the set_property_ioctl with the blob_id as value for the 
>> property
>> 8. Driver refers to the blob, gets the correction values and applies the 
>> correction in HW.
>> 9. To get currently applied color correction values, userspace:
>>  a. calls a get_property_ioctl on that color property
>>  b. gets the blob_id for the currently applied correction from DRM 
>> infrastructure
>>  c. gets the blob using get_blob_ioctl and hence the currently 
>> applied values
>>
>> That's all! :)
>>
>> About the patch series:
>> ===
>> The patch series first adds the color management support in DRM layer.
>> Then it adds the color management framework in I915 layer.
>> After that, it implements platform specific core color correction functios.
>>
>> Intel color manager registers color correction with DRM color manager in 
>> this way:
>>   - CSC transformation is registered as CTM DRM property
>>   - Gamma correction is registered as palette_after_ctm DRM property
>>   - Degamma correction is registered as palette_before_ctm DRM property
>>
>> Our thanks to all the reviewers who have given valuable comments in terms
>> of design and implementation to our previous sets of patches. Special mention
>> of thanks should go to Matt Roper for all his inputs/suggestions in 
>> implementation
>> of this module, using DRM atomic CRTC commit path.
>>
>> V2: Worked on review comments from Matt, Jim, Thierry, Rob.
>> V3: Worked on review comments from Matt, Jim, Rob:
>>   - Jim, Rob:
>> 
>> Re-arranged the whole patch series in the sequence of features, 
>> currently:
>> First 5 patches add color management support in DRM layer
>> Next 7 patches add Intel color management framework in I915 driver
>> Next 5 patches add color correction for CHV (gamma, degamma and CSC)
>> Next 2 patches enable color management, by attaching the properties to 
>> CRTC(Matt)
>> Next 4 patches add color correction for BDW (gamma, degamma)
>>   - Matt:
>> =
>> Patch 3: Added refernce/unreference for blob
>> Patch 7: return -EINVAL and added debug message
>> Patch 8: check for valid blob, from create blob
>>  moved call to intel_crtc_attach_color_prop in the end of full 
>> implementation (CHV)
>> Patch 9: DRM_ERROR->DRM_DEBUG for NULL blob case
>> Patch 13: Added static for internal functions
>> Patch 20-24: renamed gen9_* functions to bdw_*
>> Added new variables in device_info structure num_samples_after_ctm and 
>> num_samples_before_ctm
>> Added new function in patch 8 to load capabilities based on device_info 
>> across all platforms
>
> Ok I finally got around to look at the 

[Intel-gfx] [PATCH 09/23] drm/i915: Register pipe color capabilities

[Sharma, Shashank]

Regards
Shashank

On 9/22/2015 6:54 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:07:06PM +0530, Shashank Sharma wrote:
>> DRM color manager contains these color properties:
>>
>> 1. "crtc_palette_capabilities_property": to allow a
>> core driver to load and showcase its color correction
>> capabilities to user space.
>> 2. "ctm": Color transformation matrix property, where a
>> color transformation matrix of 9 correction values gets
>> applied as correction.
>> 3. "palette_before_ctm": for corrections which get applied
>> beore color transformation matrix correction.
>> 4. "palette_after_ctm": for corrections which get applied
>> after color transformation matrix correction.
>>
>> Intel color manager registers:
>> 1. Gamma correction property as "palette_after_ctm" property
>> 2. Degamma correction capability as "palette_bafore_ctm" property
>> capability as "palette_after_ctm" DRM color property hook.
>> 3. CSC as "ctm" property.
>>
>> This patch does the following:
>> 1. Add a function which loads the platform's color correction
>> capabilities in the cm_crtc_palette_capabilities_property structure.
>> 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
>> getting initiaized.
>> 3. Adds two new parameters "num_samples_after_ctm" and
>> "num_samples_before_ctm" in intel_device_info as gamma and
>> degamma coefficients vary per platform basis.
>>
>> Signed-off-by: Shashank Sharma 
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h|  2 ++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 45 
>> ++
>>   drivers/gpu/drm/i915/intel_color_manager.h |  2 ++
>>   3 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 3bf8a9b..22de2cb 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -798,6 +798,8 @@ struct intel_device_info {
>>  u8 num_sprites[I915_MAX_PIPES];
>>  u8 gen;
>>  u8 ring_mask; /* Rings supported by the HW */
>> +u16 num_samples_after_ctm;
>> +u16 num_samples_before_ctm;
>>  DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
>>  /* Register offsets for the various display pipes and transcoders */
>>  int pipe_offsets[I915_MAX_TRANSCODERS];
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 7357d99..77f58f2 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,7 +27,52 @@
>>
>>   #include "intel_color_manager.h"
>>
>> +int get_pipe_capabilities(struct drm_device *dev,
>> +struct drm_palette_caps *palette_caps, struct drm_crtc *crtc)
>> +{
>> +struct drm_property_blob *blob;
>> +
>> +/*
>> + * This function loads best capability for gamma correction
>> + * For example:
>> + * CHV best Gamma correction (CGM unit, 10 bit)
>> + * has 257 entries, best degamma is 65 entries
>> + */
>> +palette_caps->version = COLOR_STRUCT_VERSION;
>> +palette_caps->num_samples_after_ctm =
>> +INTEL_INFO(dev)->num_samples_after_ctm;
>> +palette_caps->num_samples_before_ctm =
>> +INTEL_INFO(dev)->num_samples_before_ctm;
>> +blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps),
>> +(const void *) palette_caps);
>> +if (IS_ERR_OR_NULL(blob)) {
>> +DRM_ERROR("Create blob for capabilities failed\n");
>> +return PTR_ERR(blob);
>> +}
>> +
>> +return blob->base.id;
>> +}
>> +
>>   void intel_attach_color_properties_to_crtc(struct drm_device *dev,
>>  struct drm_mode_object *mode_obj)
>>   {
>> +struct drm_mode_config *config = >mode_config;
>> +struct drm_palette_caps *palette_caps;
>> +struct drm_crtc *crtc;
>> +int capabilities_blob_id;
>> +
>> +crtc = obj_to_crtc(mode_obj);
>> +palette_caps = kzalloc(sizeof(struct drm_palette_caps),
>> +GFP_KERNEL);
>> +capabilities_blob_id = get_pipe_capabilities(dev,
>> +palette_caps, crtc);
>> +
>> +if (config->cm_crtc_palette_capabilities_property) {
>> +drm_object_attach_property(mode_obj,
>> +config->cm_crtc_palette_capabilities_property,
>> +capabilities_blob_id);
>
> I didn't find any code to attach the before/after_ctm gamma properties and
> the ctm property. Also I think a small helper would be really nice here in
> the drm core:
>
Patch 15,16,17 adds those for gamma, degamma and CSC. This is just for 
the capabilities property.
> drm_crtc_attach_cc_properties(struct drm_crtc *crtc,
> int gamma_before_ctm,
> bool ctm,
> int gamm_after_ctm)
>
> And then that directly constructs the palette caps from the given values
> (if 

[patch 4/4] drm/amdgpu: integer overflow in amdgpu_mode_dumb_create()

[Dan Carpenter]

args->size is a u64.  arg->pitch and args->height are u32.  The
multiplication will overflow instead of using the high 32 bits as
intended.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index dac14de..2023055 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -656,7 +656,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
int r;

args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) * 
((args->bpp + 1) / 8);
-   args->size = args->pitch * args->height;
+   args->size = (u64)args->pitch * args->height;
args->size = ALIGN(args->size, PAGE_SIZE);

r = amdgpu_gem_object_create(adev, args->size, 0,

[patch 3/4] drm/amdgpu: info leak in amdgpu_gem_metadata_ioctl()

[Dan Carpenter]

There is no limit on args->data.data_size_bytes so we could read beyond
the end of the args->data.data[] array.

Reported-by: Ilja Van Sprundel 
Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 5839fab..dac14de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -426,6 +426,10 @@ int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void 
*data,
   >data.data_size_bytes,
   >data.flags);
} else if (args->op == AMDGPU_GEM_METADATA_OP_SET_METADATA) {
+   if (args->data.data_size_bytes > sizeof(args->data.data)) {
+   r = -EINVAL;
+   goto unreserve;
+   }
r = amdgpu_bo_set_tiling_flags(robj, args->data.tiling_info);
if (!r)
r = amdgpu_bo_set_metadata(robj, args->data.data,
@@ -433,6 +437,7 @@ int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void 
*data,
   args->data.flags);
}

+unreserve:
amdgpu_bo_unreserve(robj);
 out:
drm_gem_object_unreference_unlocked(gobj);

[patch 2/4] drm/amdgpu: integer overflow in amdgpu_info_ioctl()

[Dan Carpenter]

The "alloc_size" calculation can overflow leading to memory corruption.

Reported-by: Ilja Van Sprundel 
Signed-off-by: Dan Carpenter 
---
The amdgpu_asic_read_register() functions seem likely to be slow.  They
iterate through all the registers to find the correct register to read.

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 2236793..8c735f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -390,7 +390,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
min((size_t)size, sizeof(vram_gtt))) ? 
-EFAULT : 0;
}
case AMDGPU_INFO_READ_MMR_REG: {
-   unsigned n, alloc_size = info->read_mmr_reg.count * 4;
+   unsigned n, alloc_size;
uint32_t *regs;
unsigned se_num = (info->read_mmr_reg.instance >>
   AMDGPU_INFO_MMR_SE_INDEX_SHIFT) &
@@ -406,9 +406,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK)
sh_num = 0x;

-   regs = kmalloc(alloc_size, GFP_KERNEL);
+   regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), 
GFP_KERNEL);
if (!regs)
return -ENOMEM;
+   alloc_size = info->read_mmr_reg.count * sizeof(*regs);

for (i = 0; i < info->read_mmr_reg.count; i++)
if (amdgpu_asic_read_register(adev, se_num, sh_num,

[patch 1/4] drm/amdgpu: unwind properly in amdgpu_cs_parser_init()

[Dan Carpenter]

The amdgpu_cs_parser_init() function doesn't clean up after itself but
instead the caller uses a free everything function amdgpu_cs_parser_fini()
on failure.  This style of error handling is often buggy.  In this
example, we call "drm_free_large(parser->chunks[i].kdata);" when it is
an unintialized pointer or when "parser->chunks" is NULL.

I fixed this bug by adding unwind code so that it frees everything that
it allocates.

I also mode some other very minor changes:
1) Renamed "r" to "ret".
2) Moved the chunk_array allocation to the start of the function.
3) Removed some initializers which are no longer needed.

Reported-by: Ilja Van Sprundel 
Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 3b355ae..abb257d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -154,42 +154,41 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
void *data)
 {
union drm_amdgpu_cs *cs = data;
uint64_t *chunk_array_user;
-   uint64_t *chunk_array = NULL;
+   uint64_t *chunk_array;
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
unsigned size, i;
-   int r = 0;
+   int ret;

-   if (!cs->in.num_chunks)
-   goto out;
+   if (cs->in.num_chunks == 0)
+   return 0;
+
+   chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t), 
GFP_KERNEL);
+   if (!chunk_array)
+   return -ENOMEM;

p->ctx = amdgpu_ctx_get(fpriv, cs->in.ctx_id);
if (!p->ctx) {
-   r = -EINVAL;
-   goto out;
+   ret = -EINVAL;
+   goto free_chunk;
}
+
p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);

/* get chunks */
INIT_LIST_HEAD(>validated);
-   chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t), 
GFP_KERNEL);
-   if (chunk_array == NULL) {
-   r = -ENOMEM;
-   goto out;
-   }
-
chunk_array_user = (uint64_t __user *)(cs->in.chunks);
if (copy_from_user(chunk_array, chunk_array_user,
   sizeof(uint64_t)*cs->in.num_chunks)) {
-   r = -EFAULT;
-   goto out;
+   ret = -EFAULT;
+   goto put_bo_list;
}

p->nchunks = cs->in.num_chunks;
p->chunks = kmalloc_array(p->nchunks, sizeof(struct amdgpu_cs_chunk),
GFP_KERNEL);
-   if (p->chunks == NULL) {
-   r = -ENOMEM;
-   goto out;
+   if (!p->chunks) {
+   ret = -ENOMEM;
+   goto put_bo_list;
}

for (i = 0; i < p->nchunks; i++) {
@@ -200,8 +199,9 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void 
*data)
chunk_ptr = (void __user *)chunk_array[i];
if (copy_from_user(_chunk, chunk_ptr,
   sizeof(struct drm_amdgpu_cs_chunk))) {
-   r = -EFAULT;
-   goto out;
+   ret = -EFAULT;
+   i--;
+   goto free_partial_kdata;
}
p->chunks[i].chunk_id = user_chunk.chunk_id;
p->chunks[i].length_dw = user_chunk.length_dw;
@@ -212,13 +212,14 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
void *data)

p->chunks[i].kdata = drm_malloc_ab(size, sizeof(uint32_t));
if (p->chunks[i].kdata == NULL) {
-   r = -ENOMEM;
-   goto out;
+   ret = -ENOMEM;
+   i--;
+   goto free_partial_kdata;
}
size *= sizeof(uint32_t);
if (copy_from_user(p->chunks[i].kdata, cdata, size)) {
-   r = -EFAULT;
-   goto out;
+   ret = -EFAULT;
+   goto free_partial_kdata;
}

switch (p->chunks[i].chunk_id) {
@@ -238,15 +239,15 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, 
void *data)
gobj = drm_gem_object_lookup(p->adev->ddev,
 p->filp, handle);
if (gobj == NULL) {
-   r = -EINVAL;
-   goto out;
+   ret = -EINVAL;
+   goto free_partial_kdata;
}

p->uf.bo = gem_to_amdgpu_bo(gobj);
p->uf.offset = fence_data->offset;
} else {
-   r = -EINVAL;
-   goto out;
+   ret = -EINVAL;
+  

[Intel-gfx] [PATCH 10/23] drm/i915: Add gamma correction handlers

[Sharma, Shashank]

Regards
Shashank

On 9/22/2015 6:45 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:07:07PM +0530, Shashank Sharma wrote:
>> I915 driver registers gamma correction as palette correction
>> property with DRM layer. This patch adds set_property() and get_property()
>> handlers for pipe level gamma correction.
>>
>> The set function attaches the Gamma correction blob to CRTC state, these
>> values will be committed during atomic commit.
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   drivers/gpu/drm/i915/intel_atomic.c| 20 
>>   drivers/gpu/drm/i915/intel_color_manager.c | 21 +
>>   drivers/gpu/drm/i915/intel_drv.h   |  5 +
>>   3 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 500d2998..0b61fef 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -315,6 +315,13 @@ int 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 = >mode_config;
>> +
>> +if (property == config->cm_palette_after_ctm_property)
>> +return intel_color_manager_set_pipe_gamma(dev, state,
>> +>base, val);
>> +
>>  DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>>  return -EINVAL;
>>   }
>> @@ -324,6 +331,19 @@ int intel_crtc_atomic_get_property(struct drm_crtc 
>> *crtc,
>> struct drm_property *property,
>> uint64_t *val)
>>   {
>> +struct drm_device *dev = crtc->dev;
>> +struct drm_mode_config *config = >mode_config;
>> +
>> +if (property == config->cm_palette_after_ctm_property) {
>> +*val = (state->palette_after_ctm_blob) ?
>> +state->palette_after_ctm_blob->base.id : 0;
>> +goto found;
>> +}
>
> Since color manager properties are meant as a new standardize KMS
> extension (we put them into the core drm_crtc_state) the get/set support
> should also be in the core. See e.g. how the rotation property is handled
> in drm_atomic_plane_get/set_property. So all this code should be added to
> drm_atomic_crtc_get/set_property.
Thanks, sounds like a good one. Will move this.
>
>
>> +
>>  DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>>  return -EINVAL;
>> +
>> +found:
>> +DRM_DEBUG_KMS("Found property %s\n", property->name);
>> +return 0;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 77f58f2..9421bb6 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,27 @@
>>
>>   #include "intel_color_manager.h"
>>
>> +int intel_color_manager_set_pipe_gamma(struct drm_device *dev,
>> +struct drm_crtc_state *crtc_state,
>> +struct drm_mode_object *obj, uint32_t blob_id)
>> +{
>> +struct drm_property_blob *blob;
>> +
>> +blob = drm_property_lookup_blob(dev, blob_id);
>> +if (!blob) {
>> +DRM_DEBUG_KMS("Invalid Blob ID\n");
>> +return -EINVAL;
>> +}
>> +
>> +if (crtc_state->palette_after_ctm_blob)
>> +drm_property_unreference_blob(
>> +crtc_state->palette_after_ctm_blob);
>> +
>> +/* Attach the blob to be committed in state */
>> +crtc_state->palette_after_ctm_blob = blob;
>> +return 0;
>> +}
>
> What is this used for? It looks a bit like legacy property code, and we
> have a generic helper to make that happen
> (drm_atomic_helper_crtc_set_property).
>
No Daniel, its not the legacy part. Please check atomic_begin() part in 
the next patches. This is like the top half of atomic set_property for 
CRTC and will be applicable for any property. As you already know, to 
set a property:
- userspace creates a blob, and sends the blob id via set_property() 
interface
- set_property() saves the blob_id in the corresponding pointer space in 
the crtc_state()
- From the atomic_commit_begin() we will extract the values from blob 
and start committing to the registers. Its kind of bottom_half() of 
atomic set_property for CRTC.
> Generally please don't add functions/structs without also adding a user,
> it means that reviewers have to constantly jump around in your patch
> series to figure out how something is used. Instead if you want to split
> things up really fine add a stub function frist (but including relevant
> callers) and then fill out the bits separately.
I think the above explanation should make it clear.
> -Daniel
>
>> +
>>   int get_pipe_capabilities(struct drm_device *dev,
>>  struct drm_palette_caps *palette_caps, struct 

4.3-rc2 on radeon: new backtraces during resume

[Pavel Machek]

HI!


I suspended T40p by mistake, and I got some lovely backtraces as a
result:

Any ideas?

Pavel

[0.00] Initializing cgroup subsys cpu
[0.00] Linux version 4.3.0-rc2+ (pavel at hobit) (gcc version 4.9.2 
(Debian 4.9.2-10) ) #111 SMP Wed Sep 23 13:29:04 CEST 2015
...
[  855.275406] PM: Syncing filesystems ... done.
[  855.745355] PM: Preparing system for sleep (mem)
[  855.830116] Freezing user space processes ... (elapsed 0.010 seconds) done.
[  855.841061] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) 
done.
[  855.844739] PM: Suspending system (mem)
[  855.844809] Suspending console(s) (use no_console_suspend to debug)
[  856.054406] parport_pc 00:07: disabled
[  856.055141] serial 00:06: disabled
[  856.056458] serial 00:06: System wakeup disabled by ACPI
[  856.058401] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[  856.059365] [ cut here ]
[  856.059381] WARNING: CPU: 0 PID: 6609 at include/drm/drm_crtc.h:1577 
drm_helper_choose_encoder_dpms+0x82/0x90()
[  856.059384] Modules linked in:
[  856.059391] CPU: 0 PID: 6609 Comm: kworker/u2:7 Not tainted 4.3.0-rc2+ #111
[  856.059393] Hardware name: IBM 2373G3U/2373G3U, BIOS 1RETDNWW (3.19 ) 
10/13/2005
[  856.059402] Workqueue: events_unbound async_run_entry_fn
[  856.059416]    de099d94 c42b40a8  de099db0 c403ecdb 
0629
[  856.059427]  c438fd82 f6acd800 f585ee00 0003 de099dc0 c403ed7f 0009 

[  856.059437]  de099dd0 c438fd82 f585ee00 f600 de099df0 c4390132 f6acd800 

[  856.059439] Call Trace:
[  856.059450]  [] dump_stack+0x41/0x59
[  856.059457]  [] warn_slowpath_common+0x6b/0xa0
[  856.059462]  [] ? drm_helper_choose_encoder_dpms+0x82/0x90
[  856.059467]  [] warn_slowpath_null+0xf/0x20
[  856.059472]  [] drm_helper_choose_encoder_dpms+0x82/0x90
[  856.059477]  [] drm_helper_connector_dpms+0x32/0xf0
[  856.059485]  [] radeon_suspend_kms+0x64/0x350
[  856.059491]  [] radeon_pmops_suspend+0x18/0x20
[  856.059497]  [] pci_pm_suspend+0x5d/0x130
[  856.059502]  [] ? pci_pm_freeze+0xc0/0xc0
[  856.059509]  [] dpm_run_callback+0x30/0x70
[  856.059514]  [] __device_suspend+0xd2/0x290
[  856.059522]  [] ? __lock_acquire.isra.24+0x3b1/0xca0
[  856.059526]  [] async_suspend+0x17/0x90
[  856.059531]  [] async_run_entry_fn+0x4f/0x140
[  856.059537]  [] process_one_work+0x15f/0x3a0
[  856.059542]  [] ? process_one_work+0x15f/0x3a0
[  856.059546]  [] ? process_one_work+0x109/0x3a0
[  856.059551]  [] worker_thread+0x39/0x430
[  856.059556]  [] ? process_one_work+0x3a0/0x3a0
[  856.059562]  [] kthread+0xae/0xd0
[  856.059572]  [] ret_from_kernel_thread+0x21/0x30
[  856.059577]  [] ? kthread_create_on_node+0x170/0x170
[  856.059581] ---[ end trace c98a514a441969f1 ]---
[  856.079439] [ cut here ]
[  856.079446] WARNING: CPU: 0 PID: 6609 at include/drm/drm_crtc.h:1577 
drm_helper_choose_crtc_dpms+0x82/0x90()
[  856.079449] Modules linked in:
[  856.079454] CPU: 0 PID: 6609 Comm: kworker/u2:7 Tainted: GW   
4.3.0-rc2+ #111
[  856.079457] Hardware name: IBM 2373G3U/2373G3U, BIOS 1RETDNWW (3.19 ) 
10/13/2005
[  856.079462] Workqueue: events_unbound async_run_entry_fn
[  856.079473]    de099d94 c42b40a8  de099db0 c403ecdb 
0629
[  856.079484]  c438fe12 f600 f6acd800 0003 de099dc0 c403ed7f 0009 

[  856.079495]  de099dd0 c438fe12 c43e24c0 f600 de099df0 c43901d6 f6acd800 

[  856.079496] Call Trace:
[  856.079503]  [] dump_stack+0x41/0x59
[  856.079508]  [] warn_slowpath_common+0x6b/0xa0
[  856.079512]  [] ? drm_helper_choose_crtc_dpms+0x82/0x90
[  856.079517]  [] warn_slowpath_null+0xf/0x20
[  856.079522]  [] drm_helper_choose_crtc_dpms+0x82/0x90
[  856.079529]  [] ? radeon_crtc_mode_fixup+0x10/0x10
[  856.079534]  [] drm_helper_connector_dpms+0xd6/0xf0
[  856.079539]  [] radeon_suspend_kms+0x64/0x350
[  856.079544]  [] radeon_pmops_suspend+0x18/0x20
[  856.079549]  [] pci_pm_suspend+0x5d/0x130
[  856.079554]  [] ? pci_pm_freeze+0xc0/0xc0
[  856.079558]  [] dpm_run_callback+0x30/0x70
[  856.079563]  [] __device_suspend+0xd2/0x290
[  856.079568]  [] ? __lock_acquire.isra.24+0x3b1/0xca0
[  856.079573]  [] async_suspend+0x17/0x90
[  856.079578]  [] async_run_entry_fn+0x4f/0x140
[  856.079583]  [] process_one_work+0x15f/0x3a0
[  856.079587]  [] ? process_one_work+0x15f/0x3a0
[  856.079592]  [] ? process_one_work+0x109/0x3a0
[  856.079597]  [] worker_thread+0x39/0x430
[  856.079601]  [] ? process_one_work+0x3a0/0x3a0
[  856.079606]  [] kthread+0xae/0xd0
[  856.079613]  [] ret_from_kernel_thread+0x21/0x30
[  856.079619]  [] ? kthread_create_on_node+0x170/0x170
[  856.079622] ---[ end trace c98a514a441969f2 ]---
[  856.139794] sd 0:0:0:0: [sda] Stopping disk
[  856.160253] radeon :01:00.0: Refused to change power state, currently in 
D0
[  856.579987] PM: suspend of devices complete after 732.935 msecs
[  

linux-next: manual merge of the akpm tree with the drm-misc tree

[Stephen Rothwell]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in:

  drivers/gpu/drm/drm_irq.c

between commit:

  4e32087d8341 ("drm: Use vblank timestamps to guesstimate how many vblanks 
were missed")

from the drm-misc tree and patch:

   "Remove abs64()"

from the akpm tree.

I fixed it up (the former removed the instances of abs64 in thet file)
and can carry the fix as necessary (no action is required).

-- 
Cheers,
Stephen Rothwellsfr at canb.auug.org.au

[Intel-gfx] [PATCH 05/23] drm: Add structure to set/get a CTM color property

[Sharma, Shashank]

Regards
Shashank

On 9/22/2015 6:38 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:07:02PM +0530, Shashank Sharma wrote:
>> From: Kausal Malladi 
>>
>> Color Manager framework defines a color correction property for color
>> space transformation and Gamut mapping. This property is called CTM (Color
>> Transformation Matrix).
>>
>> This patch adds a new structure in DRM layer for CTM.
>> This structure can be used by all user space agents to
>> configure CTM coefficients for color correction.
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   include/uapi/drm/drm.h | 12 
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index f72b916..9580772 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -867,6 +867,18 @@ struct drm_palette {
>>  struct drm_r32g32b32 lut[0];
>>   };
>>
>> +struct drm_ctm {
>> +/* Structure version. Should be 1 currently */
>> +__u32 version;
>
> Same thing here, no version needed for properties.
Agree.
-Shashank
>
>> +/*
>> + * Each value is in S31.32 format.
>> + * This is 3x3 matrix in row major format.
>> + * Integer part will be clipped to nearest
>> + * max/min boundary as supported by the HW platform.
>> + */
>> +__s64 ctm_coeff[9];
>> +};
>> +
>>   /* typedef area */
>>   #ifndef __KERNEL__
>>   typedef struct drm_clip_rect drm_clip_rect_t;
>> --
>> 1.9.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[PATCH 04/23] drm: Add drm structures for palette color property

[Sharma, Shashank]

Regards
Shashank

On 9/22/2015 6:38 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:07:01PM +0530, Shashank Sharma wrote:
>> From: Kausal Malladi 
>>
>> This patch adds new structures in DRM layer for Palette color
>> correction.These structures will be used by user space agents
>> to configure appropriate number of samples and Palette LUT for
>> a platform.
>>
>> Signed-off-by: Shashank Sharma 
>> Signed-off-by: Kausal Malladi 
>> ---
>>   include/uapi/drm/drm.h | 27 +++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index e3c642f..f72b916 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -840,6 +840,33 @@ struct drm_palette_caps {
>>  __u32 num_samples_after_ctm;
>>   };
>>
>> +struct drm_r32g32b32 {
>> +/*
>> + * Data is in U8.24 fixed point format.
>> + * All platforms support values within [0, 1.0] range,
>> + * for Red, Green and Blue colors.
>> + */
>> +__u32 r32;
>> +__u32 g32;
>> +__u32 b32;
>
> It's not strictly required, but adding a __u32 reserved here to align the
> struct to 64 bits seems good imo. Slight overhead but meh about that.
Humm, ok, we can check this out.
>
>> +};
>> +
>> +struct drm_palette {
>> +/* Structure version. Should be 1 currently */
>> +__u32 version;
>
> Definitely great practice to take compat into account and definitely
> needed for the first design using ioctls but I don't think we need this
> here. Properties are already extinsible themselves: We can just greate a
> "ctm-v2", "ctm-v3" if the layout changes, and since the actual ctm matrix
> is stored in the drm_crtc_state any compat code on the kernel will be
> shared.
>
> Aside: For an ioctl the recommended way to handle backwards compat and
> extensions in drm is with a flags bitfield. That's more flexible than a
> linear version field, and extending the ioctl struct at the end is already
> handled by the drm core in a transparent fashion (it 0-fills either kernel
> or userspace side).
>
Agree, we will drop this. Do you think we should add a flags field, or 
is it ok without it ?
>> +/*
>> + * This has to be a supported value during get call.
>> + * Feature will be disabled if this is 0 while set
>> + */
>> +__u32 num_samples;
>
> blob properties already have a size, storing it again in the blob is
> redundnant. Instead I think a small helper to get the number of samples
> for a given gamma table blob would be needed.
>
> Cheers, Daniel
Please note that they are different. One is the size of blob and other 
one is the num_samples supported by the property, in the current 
correction mode. If you check the design doc, num_sample serves the 
purpose of deciding which correction mode to be applied also. fox ex, 
for gamma, num_samples=0 indicates disable gamma, whereas 
num_samples=512 indicates split gamma mode.

Shashank
>
>> +/*
>> + * Starting of palette LUT in R32G32B32 format.
>> + * Each of RGB value is in U8.24 fixed point format.
>> + * Actual number of samples will depend upon num_samples
>> + */
>> +struct drm_r32g32b32 lut[0];
>> +};
>> +
>>   /* typedef area */
>>   #ifndef __KERNEL__
>>   typedef struct drm_clip_rect drm_clip_rect_t;
>> --
>> 1.9.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

--- Comment #2 from Balázs Vinarz  ---
Created attachment 118413
  --> https://bugs.freedesktop.org/attachment.cgi?id=118413=edit
dmesg.full

-- 
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/20150923/89edf304/attachment.html>

[PATCH 02/23] drm: Add structure for querying palette color capabilities

[Sharma, Shashank]

Hi Matt, Daniel
Addressing the review comments from both of you here.

Regards
Shashank

On 9/22/2015 6:32 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 10:51:31AM -0700, Matt Roper wrote:
>> On Wed, Sep 16, 2015 at 11:06:59PM +0530, Shashank Sharma wrote:
>>> From: Kausal Malladi 
>>>
>>> The DRM color management framework is targeting various hardware
>>> platforms and drivers. Different platforms can have different color
>>> correction and enhancement capabilities.
>>>
>>> A commom user space application can query these capabilities using the
>>> DRM property interface. Each driver can fill this property with its
>>> platform's palette color capabilities.
>>>
>>> This patch adds new structure in DRM layer for querying palette color
>>> capabilities. This structure will be used by all user space
>>> agents to configure appropriate color configurations.
>>>
>>> Signed-off-by: Shashank Sharma 
>>> Signed-off-by: Kausal Malladi 
>>
>> I think you provided an explanation on a previous code review cycle, but
>> I forget the details now...what's the benefit to using a blob for caps
>> rather than having these be individual properties?  Individual
>> properties seems more natural to me, but I think you had a justification
>> for blobbing them together; that reasoning would be good to include in
>> the commit message.
>
> Yeah I'm leaning slightly towards individual props too, that would give us
> a bit more freedom with placing them (e.g. if someone comes up with funky
> hw where before_ctm and ctm are per-plane and after_ctm is on the crtc,
> with only some planes support the before_ctm gamma table).
This was the part where we spent most of the time during the design 
review, and the reason we came up for this was:
- This is a read only property, which userspace would like to read only 
once, and cache the information. It was also Gary's opinion to keep this 
as single blob for all.
- Making individual property needs more information to be provided to
user space.
- This is a blob only for pipe level capabilities, the plane level blob 
will be separate from this.
- We can handle this HW also, by loading proper plane and pipe level 
capability blob. This is more convenient to have all the capabilities 
together at the same place, than keep on querying the same.
>
> Also if you do per-prop properties instead of the blob you can drop the
> version/reserved fields, since properties are inheritedly designed to be
> extendible. So no need to revision them again (it only leads to more code
> that might break).
> -Daniel
>
We are anyways planning to drop the version, as per Ville's comment.
- Shashank
>>
>>
>> Matt
>>
>>> ---
>>>   include/uapi/drm/drm.h | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>>> index 3801584..e3c642f 100644
>>> --- a/include/uapi/drm/drm.h
>>> +++ b/include/uapi/drm/drm.h
>>> @@ -829,6 +829,17 @@ struct drm_event_vblank {
>>> __u32 reserved;
>>>   };
>>>
>>> +struct drm_palette_caps {
>>> +   /* Structure version. Should be 1 currently */
>>> +   __u32 version;
>>> +   /* For padding and future use */
>>> +   __u32 reserved;
>>> +   /* This may be 0 if not supported. e.g. plane palette or VLV pipe */
>>> +   __u32 num_samples_before_ctm;
>>> +   /* This will be non-zero for pipe. May be zero for planes on some HW */
>>> +   __u32 num_samples_after_ctm;
>>> +};
>>> +
>>>   /* typedef area */
>>>   #ifndef __KERNEL__
>>>   typedef struct drm_clip_rect drm_clip_rect_t;
>>> --
>>> 1.9.1
>>>
>>
>> --
>> Matt Roper
>> Graphics Software Engineer
>> IoTG Platform Enabling & Development
>> Intel Corporation
>> (916) 356-2795
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[PATCH] libdrm: Use userspace compatible type in fourcc_mod_code macro

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

__u64 should be used instead of u64.

Kernel headers originally pulled in:

commit 8983fe5497e89a3ffaba3ad1ee06a30a1c7e6daf
Author: Tvrtko Ursulin 
Date:   Mon Aug 3 10:48:03 2015 +0100

libdrm: Add framebuffer modifiers uapi

Signed-off-by: Tvrtko Ursulin 
Cc: dri-devel at lists.freedesktop.org
Cc: Rob Clark 
Cc: Daniel Vetter 
---
 include/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 63a80ca5ba39..e741b09a00fd 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -149,7 +149,7 @@
 /* add more to the end as needed */

 #define fourcc_mod_code(vendor, val) \
-   u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
0x00ffULL))
+   __u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
0x00ffULL))

 /*
  * Format Modifier tokens:
-- 
2.5.1

linux-next: build warning after merge of the drm-misc tree

[Stephen Rothwell]

Hi all,

After merging the drm-misc tree, today's linux-next build (arm
multi_v7_defconfig) produced this warning:

drivers/gpu/drm/drm_crtc.c: In function 'drm_fb_release':
drivers/gpu/drm/drm_crtc.c:3494:21: warning: unused variable 'dev' 
[-Wunused-variable]
  struct drm_device *dev = priv->minor->dev;
 ^

Introduced by commit

  3d2e74c94432 ("drm/core: Preserve the fb id on close.")

-- 
Cheers,
Stephen Rothwellsfr at canb.auug.org.au

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

--- Comment #1 from Alex Deucher  ---
Please attach your xorg log and dmesg output.

-- 
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/20150923/f786fc03/attachment.html>

[patch 1/4] drm/amdgpu: unwind properly in amdgpu_cs_parser_init()

[Alex Deucher]

On Wed, Sep 23, 2015 at 10:16 AM, Christian König
 wrote:
> On 23.09.2015 12:59, Dan Carpenter wrote:
>>
>> The amdgpu_cs_parser_init() function doesn't clean up after itself but
>> instead the caller uses a free everything function amdgpu_cs_parser_fini()
>> on failure.  This style of error handling is often buggy.  In this
>> example, we call "drm_free_large(parser->chunks[i].kdata);" when it is
>> an unintialized pointer or when "parser->chunks" is NULL.
>>
>> I fixed this bug by adding unwind code so that it frees everything that
>> it allocates.
>>
>> I also mode some other very minor changes:
>> 1) Renamed "r" to "ret".
>> 2) Moved the chunk_array allocation to the start of the function.
>> 3) Removed some initializers which are no longer needed.
>>
>> Reported-by: Ilja Van Sprundel 
>> Signed-off-by: Dan Carpenter 
>
>
> The whole set looks sane to me, patches are Reviewed-by: Christian König
> 

Applied.  thanks!

Alex

>
> Regards,
> Christian.
>
>
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 3b355ae..abb257d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -154,42 +154,41 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser
>> *p, void *data)
>>   {
>> union drm_amdgpu_cs *cs = data;
>> uint64_t *chunk_array_user;
>> -   uint64_t *chunk_array = NULL;
>> +   uint64_t *chunk_array;
>> struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>> unsigned size, i;
>> -   int r = 0;
>> +   int ret;
>>   - if (!cs->in.num_chunks)
>> -   goto out;
>> +   if (cs->in.num_chunks == 0)
>> +   return 0;
>> +
>> +   chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t),
>> GFP_KERNEL);
>> +   if (!chunk_array)
>> +   return -ENOMEM;
>> p->ctx = amdgpu_ctx_get(fpriv, cs->in.ctx_id);
>> if (!p->ctx) {
>> -   r = -EINVAL;
>> -   goto out;
>> +   ret = -EINVAL;
>> +   goto free_chunk;
>> }
>> +
>> p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
>> /* get chunks */
>> INIT_LIST_HEAD(>validated);
>> -   chunk_array = kmalloc_array(cs->in.num_chunks, sizeof(uint64_t),
>> GFP_KERNEL);
>> -   if (chunk_array == NULL) {
>> -   r = -ENOMEM;
>> -   goto out;
>> -   }
>> -
>> chunk_array_user = (uint64_t __user *)(cs->in.chunks);
>> if (copy_from_user(chunk_array, chunk_array_user,
>>sizeof(uint64_t)*cs->in.num_chunks)) {
>> -   r = -EFAULT;
>> -   goto out;
>> +   ret = -EFAULT;
>> +   goto put_bo_list;
>> }
>> p->nchunks = cs->in.num_chunks;
>> p->chunks = kmalloc_array(p->nchunks, sizeof(struct
>> amdgpu_cs_chunk),
>> GFP_KERNEL);
>> -   if (p->chunks == NULL) {
>> -   r = -ENOMEM;
>> -   goto out;
>> +   if (!p->chunks) {
>> +   ret = -ENOMEM;
>> +   goto put_bo_list;
>> }
>> for (i = 0; i < p->nchunks; i++) {
>> @@ -200,8 +199,9 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p,
>> void *data)
>> chunk_ptr = (void __user *)chunk_array[i];
>> if (copy_from_user(_chunk, chunk_ptr,
>>sizeof(struct
>> drm_amdgpu_cs_chunk))) {
>> -   r = -EFAULT;
>> -   goto out;
>> +   ret = -EFAULT;
>> +   i--;
>> +   goto free_partial_kdata;
>> }
>> p->chunks[i].chunk_id = user_chunk.chunk_id;
>> p->chunks[i].length_dw = user_chunk.length_dw;
>> @@ -212,13 +212,14 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser
>> *p, void *data)
>> p->chunks[i].kdata = drm_malloc_ab(size,
>> sizeof(uint32_t));
>> if (p->chunks[i].kdata == NULL) {
>> -   r = -ENOMEM;
>> -   goto out;
>> +   ret = -ENOMEM;
>> +   i--;
>> +   goto free_partial_kdata;
>> }
>> size *= sizeof(uint32_t);
>> if (copy_from_user(p->chunks[i].kdata, cdata, size)) {
>> -   r = -EFAULT;
>> -   goto out;
>> +   ret = -EFAULT;
>> +   goto free_partial_kdata;
>> }
>> switch (p->chunks[i].chunk_id) {
>> @@ -238,15 +239,15 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser
>> *p, void *data)
>> gobj =
>> drm_gem_object_lookup(p->adev->ddev,
>>  p->filp,
>> handle);
>>   

[PATCH 5/5] ASoC: AMD: add AMD ASoC ACP-I2S driver

[Alex Deucher]

From: Maruthi Srinivas Bayyavarapu 

ACP IP block consists of dedicated DMA and I2S blocks. The PCM driver
provides the platform DMA component to ALSA core.

Signed-off-by: Maruthi Bayyavarapu 
Reviewed-by: Alex Deucher 
Reviewed-by: Murali Krishna Vemuri 
---

v2: squash in Kconfig fix
v3: squash additional commits, convert to mfd, drop rt286 changes
v4: add major changes as below:
1. remove i2s specific changes and add them to dwc i2s driver.
2. add ACP DMA logic to PCM driver.

 sound/soc/Kconfig   |   1 +
 sound/soc/Makefile  |   1 +
 sound/soc/amd/Kconfig   |   4 +
 sound/soc/amd/Makefile  |   3 +
 sound/soc/amd/acp-pcm-dma.c | 518 +++
 sound/soc/amd/acp.c | 736 
 sound/soc/amd/acp.h | 147 +
 7 files changed, 1410 insertions(+)
 create mode 100644 sound/soc/amd/Kconfig
 create mode 100644 sound/soc/amd/Makefile
 create mode 100644 sound/soc/amd/acp-pcm-dma.c
 create mode 100644 sound/soc/amd/acp.c
 create mode 100644 sound/soc/amd/acp.h

diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 1d651b8..115264c 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -35,6 +35,7 @@ config SND_SOC_TOPOLOGY

 # All the supported SoCs
 source "sound/soc/adi/Kconfig"
+source "sound/soc/amd/Kconfig"
 source "sound/soc/atmel/Kconfig"
 source "sound/soc/au1x/Kconfig"
 source "sound/soc/bcm/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 669648b..9400ec3 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_SND_SOC) += snd-soc-core.o
 obj-$(CONFIG_SND_SOC)  += codecs/
 obj-$(CONFIG_SND_SOC)  += generic/
 obj-$(CONFIG_SND_SOC)  += adi/
+obj-$(CONFIG_SND_SOC)  += amd/
 obj-$(CONFIG_SND_SOC)  += atmel/
 obj-$(CONFIG_SND_SOC)  += au1x/
 obj-$(CONFIG_SND_SOC)  += bcm/
diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
new file mode 100644
index 000..78187eb
--- /dev/null
+++ b/sound/soc/amd/Kconfig
@@ -0,0 +1,4 @@
+config SND_SOC_AMD_ACP
+   tristate "AMD Audio Coprocessor support"
+   help
+This option enables ACP DMA support on AMD platform.
diff --git a/sound/soc/amd/Makefile b/sound/soc/amd/Makefile
new file mode 100644
index 000..62648cb
--- /dev/null
+++ b/sound/soc/amd/Makefile
@@ -0,0 +1,3 @@
+snd-soc-acp-pcm-objs   := acp-pcm-dma.o acp.o
+
+obj-$(CONFIG_SND_SOC_AMD_ACP) += snd-soc-acp-pcm.o
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
new file mode 100644
index 000..5044188
--- /dev/null
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -0,0 +1,518 @@
+/*
+ * AMD ALSA SoC PCM Driver
+ *
+ * Copyright 2014-2015 Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "acp.h"
+
+#define PLAYBACK_MIN_NUM_PERIODS2
+#define PLAYBACK_MAX_NUM_PERIODS2
+#define PLAYBACK_MAX_PERIOD_SIZE16384
+#define PLAYBACK_MIN_PERIOD_SIZE1024
+#define CAPTURE_MIN_NUM_PERIODS 2
+#define CAPTURE_MAX_NUM_PERIODS 2
+#define CAPTURE_MAX_PERIOD_SIZE 16384
+#define CAPTURE_MIN_PERIOD_SIZE 1024
+
+#define NUM_DSCRS_PER_CHANNEL 2
+
+#define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS)
+#define MIN_BUFFER MAX_BUFFER
+
+static const struct snd_pcm_hardware acp_pcm_hardware_playback = {
+   .info = SNDRV_PCM_INFO_INTERLEAVED |
+   SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP |
+   SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH |
+   SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME,
+   .formats = SNDRV_PCM_FMTBIT_S16_LE |
+   SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
+   .channels_min = 1,
+   .channels_max = 8,
+   .rates = SNDRV_PCM_RATE_8000_96000,
+   .rate_min = 8000,
+   .rate_max = 96000,
+   .buffer_bytes_max = PLAYBACK_MAX_NUM_PERIODS * PLAYBACK_MAX_PERIOD_SIZE,
+   .period_bytes_min = PLAYBACK_MIN_PERIOD_SIZE,
+   .period_bytes_max = PLAYBACK_MAX_PERIOD_SIZE,
+   .periods_min = PLAYBACK_MIN_NUM_PERIODS,
+   .periods_max = PLAYBACK_MAX_NUM_PERIODS,
+};
+
+static const struct snd_pcm_hardware acp_pcm_hardware_capture = {
+   .info = SNDRV_PCM_INFO_INTERLEAVED |
+   SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP |
+   SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH |
+   

[PATCH 3/5] drm/amd: add ACP driver support

[Alex Deucher]

From: Maruthi Bayyavarapu 

This adds the ACP (Audio CoProcessor) IP driver and wires
it up to the amdgpu driver.  The ACP block provides the DMA
engine for i2s based ALSA driver. This is required for audio
on APUs that utilize an i2s codec.

Reviewed-by: Jammy Zhou 
Reviewed-by: Maruthi Bayyavarapu 
Reviewed-by: Alex Deucher 
Reviewed-by: Murali Krishna Vemuri 
Signed-off-by: Maruthi Bayyavarapu 
Signed-off-by: Chunming Zhou 
Signed-off-by: Alex Deucher 
---

v2: integrate i2s/az check patch
v3: s/amd_acp/amdgpu_acp/
v4: update copyright notice
v5: squash multiple patches, convert to mfd
v6: major changes as below :
1. Pass ACP register base to DMA and dw i2s drivers
   as IORESOURCE_MEM resources.
2. add dw i2s as a new mfd cell.

 drivers/gpu/drm/Kconfig  |   2 +
 drivers/gpu/drm/amd/acp/Kconfig  |  10 +
 drivers/gpu/drm/amd/acp/Makefile |   9 +
 drivers/gpu/drm/amd/acp/acp_hw.c | 127 +
 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h |  49 +
 drivers/gpu/drm/amd/amdgpu/Makefile  |  13 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  12 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c  | 269 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h  |  41 
 drivers/gpu/drm/amd/amdgpu/vi.c  |  12 ++
 drivers/gpu/drm/amd/include/amd_shared.h |   1 +
 include/linux/mfd/amd_acp.h  |  43 +
 12 files changed, 587 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd/acp/Kconfig
 create mode 100644 drivers/gpu/drm/amd/acp/Makefile
 create mode 100644 drivers/gpu/drm/amd/acp/acp_hw.c
 create mode 100644 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h
 create mode 100644 include/linux/mfd/amd_acp.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 1a0a8df..330e9fb 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -161,6 +161,8 @@ config DRM_AMDGPU

 source "drivers/gpu/drm/amd/amdgpu/Kconfig"

+source "drivers/gpu/drm/amd/acp/Kconfig"
+
 source "drivers/gpu/drm/nouveau/Kconfig"

 config DRM_I810
diff --git a/drivers/gpu/drm/amd/acp/Kconfig b/drivers/gpu/drm/amd/acp/Kconfig
new file mode 100644
index 000..1de4fe7
--- /dev/null
+++ b/drivers/gpu/drm/amd/acp/Kconfig
@@ -0,0 +1,10 @@
+menu "ACP Configuration"
+
+config DRM_AMD_ACP
+   bool "Enable ACP IP support"
+   default y
+   depends on MFD_CORE
+   help
+   Choose this option to enable ACP IP support for AMD SOCs.
+
+endmenu
diff --git a/drivers/gpu/drm/amd/acp/Makefile b/drivers/gpu/drm/amd/acp/Makefile
new file mode 100644
index 000..c8c3303
--- /dev/null
+++ b/drivers/gpu/drm/amd/acp/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for the ACP, which is a sub-component
+# of AMDSOC/AMDGPU drm driver.
+# It provides the HW control for ACP related functionalities.
+
+ccflags-y += -Idrivers/gpu/drm/amd/include/asic_reg/acp
+subdir-ccflags-y += -I$(AMDACPPATH)/ -I$(AMDACPPATH)/include
+
+AMD_ACP_FILES := $(AMDACPPATH)/acp_hw.o
diff --git a/drivers/gpu/drm/amd/acp/acp_hw.c b/drivers/gpu/drm/amd/acp/acp_hw.c
new file mode 100644
index 000..55220c3
--- /dev/null
+++ b/drivers/gpu/drm/amd/acp/acp_hw.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright 2015 Advanced Micro Devices, Inc.
+ *
+ * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "acp_gfx_if.h"
+
+#define ACP_MODE_I2S   0
+#define ACP_MODE_AZ1
+
+#define VISLANDS30_IV_SRCID_ACP 0x00a2
+#define mmACP_AZALIA_I2S_SELECT 0x51d4
+
+static int irq_set_source(void *private_data, unsigned src_id, unsigned type,
+   int enabled)
+{
+   struct acp_irq_prv *idata = private_data;
+
+   if 

[PATCH 2/5] ASoC : dwc : support dw i2s in AMD platform

[Alex Deucher]

From: Maruthi Srinivas Bayyavarapu 

Vendor specific quirk was added to:
1. Support AMD platform which has two dwc controllers with different
   base address for playback and capture. Also, I2S_COMP_PARAM_*
   registers offsets differed.
2. Resume audio which was active before system suspend.
   After 'resume', dwc need to be reconfigured with params
   configured during 'hw_params' callback. With this, audio usecase
   continues from where it got stopped because of 'suspend'.

Signed-off-by: Maruthi Bayyavarapu 
---
 include/sound/designware_i2s.h |   3 +
 sound/soc/dwc/designware_i2s.c | 190 +++--
 2 files changed, 127 insertions(+), 66 deletions(-)

diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h
index 08e3418..546d1a3 100644
--- a/include/sound/designware_i2s.h
+++ b/include/sound/designware_i2s.h
@@ -43,6 +43,9 @@ struct i2s_platform_data {
int channel;
u32 snd_fmts;
u32 snd_rates;
+   #define DW_I2S_VENDOR_AMD (1 << 0)
+   u32 quirks;
+

void *play_dma_data;
void *capture_dma_data;
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index e3dcaca..cade67e 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -89,11 +89,14 @@ union dw_i2s_snd_dma_data {
 };

 struct dw_i2s_dev {
-   void __iomem *i2s_base;
+   void __iomem *i2s_pbase;
+   void __iomem *i2s_cbase;
struct clk *clk;
int active;
unsigned int capability;
struct device *dev;
+   u32 ccr;
+   u32 xfer_resolution;

/* data related to DMA transfers b/w i2s and DMAC */
union dw_i2s_snd_dma_data play_dma_data;
@@ -118,10 +121,10 @@ static inline void i2s_disable_channels(struct dw_i2s_dev 
*dev, u32 stream)

if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
for (i = 0; i < 4; i++)
-   i2s_write_reg(dev->i2s_base, TER(i), 0);
+   i2s_write_reg(dev->i2s_pbase, TER(i), 0);
} else {
for (i = 0; i < 4; i++)
-   i2s_write_reg(dev->i2s_base, RER(i), 0);
+   i2s_write_reg(dev->i2s_cbase, RER(i), 0);
}
 }

@@ -131,25 +134,25 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, 
u32 stream)

if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
for (i = 0; i < 4; i++)
-   i2s_write_reg(dev->i2s_base, TOR(i), 0);
+   i2s_write_reg(dev->i2s_pbase, TOR(i), 0);
} else {
for (i = 0; i < 4; i++)
-   i2s_write_reg(dev->i2s_base, ROR(i), 0);
+   i2s_write_reg(dev->i2s_cbase, ROR(i), 0);
}
 }

 static void i2s_start(struct dw_i2s_dev *dev,
  struct snd_pcm_substream *substream)
 {
-
-   i2s_write_reg(dev->i2s_base, IER, 1);
-
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-   i2s_write_reg(dev->i2s_base, ITER, 1);
-   else
-   i2s_write_reg(dev->i2s_base, IRER, 1);
-
-   i2s_write_reg(dev->i2s_base, CER, 1);
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   i2s_write_reg(dev->i2s_pbase, IER, 1);
+   i2s_write_reg(dev->i2s_pbase, ITER, 1);
+   i2s_write_reg(dev->i2s_pbase, CER, 1);
+   } else {
+   i2s_write_reg(dev->i2s_cbase, IER, 1);
+   i2s_write_reg(dev->i2s_cbase, IRER, 1);
+   i2s_write_reg(dev->i2s_cbase, CER, 1);
+   }
 }

 static void i2s_stop(struct dw_i2s_dev *dev,
@@ -157,26 +160,40 @@ static void i2s_stop(struct dw_i2s_dev *dev,
 {
u32 i = 0, irq;

+   const struct i2s_platform_data *pdata = dev_get_platdata(dev->dev);
+
i2s_clear_irqs(dev, substream->stream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   i2s_write_reg(dev->i2s_base, ITER, 0);
-
+   i2s_write_reg(dev->i2s_pbase, ITER, 0);
for (i = 0; i < 4; i++) {
-   irq = i2s_read_reg(dev->i2s_base, IMR(i));
-   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x30);
+   irq = i2s_read_reg(dev->i2s_pbase, IMR(i));
+   i2s_write_reg(dev->i2s_pbase, IMR(i),
+   irq | 0x30);
}
} else {
-   i2s_write_reg(dev->i2s_base, IRER, 0);
+   i2s_write_reg(dev->i2s_cbase, IRER, 0);

for (i = 0; i < 4; i++) {
-   irq = i2s_read_reg(dev->i2s_base, IMR(i));
-   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x03);
+   irq = i2s_read_reg(dev->i2s_cbase, IMR(i));
+   i2s_write_reg(dev->i2s_cbase, IMR(i),
+   irq | 0x03);
}
}

-   if 

[PATCH 1/5] ASoC : dwc : support dw i2s in slave mode

[Alex Deucher]

From: Maruthi Srinivas Bayyavarapu 

dw i2s controller can work in slave mode, codec being master.
Added a device caps (DW_I2S_SLAVE) to support slave mode operation.
This can be added to platform data to support slave mode.

Signed-off-by: Maruthi Bayyavarapu 
---
 include/sound/designware_i2s.h |  1 +
 sound/soc/dwc/designware_i2s.c | 86 --
 2 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h
index 3a8fca9..08e3418 100644
--- a/include/sound/designware_i2s.h
+++ b/include/sound/designware_i2s.h
@@ -38,6 +38,7 @@ struct i2s_clk_config_data {
 struct i2s_platform_data {
#define DWC_I2S_PLAY(1 << 0)
#define DWC_I2S_RECORD  (1 << 1)
+   #define DW_I2S_SLAVE   (1 << 2)
unsigned int cap;
int channel;
u32 snd_fmts;
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index a3e97b4..e3dcaca 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -209,6 +209,7 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
struct i2s_clk_config_data *config = >config;
+   const struct i2s_platform_data *pdata = dev_get_platdata(dai->dev);
u32 ccr, xfer_resolution, ch_reg, irq;
int ret;

@@ -273,23 +274,25 @@ static int dw_i2s_hw_params(struct snd_pcm_substream 
*substream,

config->sample_rate = params_rate(params);

-   if (dev->i2s_clk_cfg) {
-   ret = dev->i2s_clk_cfg(config);
-   if (ret < 0) {
-   dev_err(dev->dev, "runtime audio clk config fail\n");
-   return ret;
+   if (!(pdata->cap & DW_I2S_SLAVE)) {
+   if (dev->i2s_clk_cfg) {
+   ret = dev->i2s_clk_cfg(config);
+   if (ret < 0) {
+   dev_err(dev->dev, "runtime audio clk config 
fail\n");
+   return ret;
}
-   } else {
-   u32 bitclk = config->sample_rate * config->data_width * 2;
-
-   ret = clk_set_rate(dev->clk, bitclk);
-   if (ret) {
-   dev_err(dev->dev, "Can't set I2S clock rate: %d\n",
-   ret);
-   return ret;
+   } else {
+   u32 bitclk = config->sample_rate *
+   config->data_width * 2;
+
+   ret = clk_set_rate(dev->clk, bitclk);
+   if (ret) {
+   dev_err(dev->dev, "Can't set I2S clock rate: 
%d\n",
+   ret);
+   return ret;
+   }
}
}
-
return 0;
 }

@@ -356,16 +359,20 @@ static const struct snd_soc_component_driver 
dw_i2s_component = {
 static int dw_i2s_suspend(struct snd_soc_dai *dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+   const struct i2s_platform_data *pdata = dev_get_platdata(dai->dev);

-   clk_disable(dev->clk);
+   if (!(pdata->cap & DW_I2S_SLAVE))
+   clk_disable(dev->clk);
return 0;
 }

 static int dw_i2s_resume(struct snd_soc_dai *dai)
 {
struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
+   const struct i2s_platform_data *pdata = dev_get_platdata(dai->dev);

-   clk_enable(dev->clk);
+   if (!(pdata->cap & DW_I2S_SLAVE))
+   clk_enable(dev->clk);
return 0;
 }

@@ -529,6 +536,7 @@ static int dw_i2s_probe(struct platform_device *pdev)
struct resource *res;
int ret;
struct snd_soc_dai_driver *dw_i2s_dai;
+   const char *clk_id;

dev = devm_kzalloc(>dev, sizeof(*dev), GFP_KERNEL);
if (!dev) {
@@ -550,33 +558,36 @@ static int dw_i2s_probe(struct platform_device *pdev)
return PTR_ERR(dev->i2s_base);

dev->dev = >dev;
+
if (pdata) {
+   clk_id = NULL;
ret = dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata);
-   if (ret < 0)
-   return ret;
-
dev->capability = pdata->cap;
-   dev->i2s_clk_cfg = pdata->i2s_clk_cfg;
-   if (!dev->i2s_clk_cfg) {
-   dev_err(>dev, "no clock configure method\n");
-   return -ENODEV;
-   }
-
-   dev->clk = devm_clk_get(>dev, NULL);
} else {
+   clk_id = "i2sclk";
ret = dw_configure_dai_by_dt(dev, dw_i2s_dai, res);
-   if (ret < 0)
-   return ret;
-
-   dev->clk = devm_clk_get(>dev, "i2sclk");
}
-   if (IS_ERR(dev->clk))
-   return PTR_ERR(dev->clk);
-
-   ret = 

[PATCH 0/5] Add ASoC support for AMD APUs [v3]

[Alex Deucher]

This patch set implements support for i2s audio and new AMD GPUs.
The i2s codec is fed by a DMA engine on the GPU.  To handle this
we create mfd cells which we hang the i2s codec and DMA engine on.
Because of this, this patch set covers two subsystems: drm and alsa.
The drm patches add support for the ACP hw block which provides the
DMA engine for the i2s codec.  The alsa patches add the ASoC driver
for the i2s codec.  Since the alsa changes depend on the drm changes
in this patch set as well as some other drm changes queued for 4.3,
I'd like to take the alsa patches in via the drm tree.

V2 changes:
- Use the MFD subsystem rather than adding our own bus
- Squash all sub-feature patches together
- fix comments mentioned in previous review

V3 changes:
- Update the designware driver to handle slave mode, amd specific
  features
- Use the designware driver directly for i2s
- Move the DMA handling from the GPU driver into the AMD ASoC
  driver
- Change the license on the ASoC driver to GPL

Patch 4 adds the register headers for the ACP block which is a
pretty big patch so I've excluded it from email.  The entire patch
set can be viewed here:
http://cgit.freedesktop.org/~agd5f/linux/log/?h=acp-upstream3

Thanks,

Alex

Maruthi Bayyavarapu (1):
  drm/amd: add ACP driver support

Maruthi Srinivas Bayyavarapu (4):
  ASoC : dwc : support dw i2s in slave mode
  ASoC : dwc : support dw i2s in AMD platform
  ASoC : AMD : add ACP 2.2 register headers
  ASoC: AMD: add AMD ASoC ACP-I2S driver

 drivers/gpu/drm/Kconfig  |2 +
 drivers/gpu/drm/amd/acp/Kconfig  |   10 +
 drivers/gpu/drm/amd/acp/Makefile |9 +
 drivers/gpu/drm/amd/acp/acp_hw.c |  127 ++
 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h |   49 +
 drivers/gpu/drm/amd/amdgpu/Makefile  |   13 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |   12 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c  |  269 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h  |   41 +
 drivers/gpu/drm/amd/amdgpu/vi.c  |   12 +
 drivers/gpu/drm/amd/include/amd_shared.h |1 +
 include/linux/mfd/amd_acp.h  |   43 +
 include/sound/designware_i2s.h   |4 +
 sound/soc/Kconfig|1 +
 sound/soc/Makefile   |1 +
 sound/soc/amd/Kconfig|4 +
 sound/soc/amd/Makefile   |3 +
 sound/soc/amd/acp-pcm-dma.c  |  518 ++
 sound/soc/amd/acp.c  |  736 +
 sound/soc/amd/acp.h  |  147 ++
 sound/soc/amd/include/acp_2_2_d.h|  609 +++
 sound/soc/amd/include/acp_2_2_enum.h | 1068 
 sound/soc/amd/include/acp_2_2_sh_mask.h  | 2292 ++
 sound/soc/dwc/designware_i2s.c   |  270 +--
 24 files changed, 6141 insertions(+), 100 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/acp/Kconfig
 create mode 100644 drivers/gpu/drm/amd/acp/Makefile
 create mode 100644 drivers/gpu/drm/amd/acp/acp_hw.c
 create mode 100644 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h
 create mode 100644 include/linux/mfd/amd_acp.h
 create mode 100644 sound/soc/amd/Kconfig
 create mode 100644 sound/soc/amd/Makefile
 create mode 100644 sound/soc/amd/acp-pcm-dma.c
 create mode 100644 sound/soc/amd/acp.c
 create mode 100644 sound/soc/amd/acp.h
 create mode 100644 sound/soc/amd/include/acp_2_2_d.h
 create mode 100644 sound/soc/amd/include/acp_2_2_enum.h
 create mode 100644 sound/soc/amd/include/acp_2_2_sh_mask.h

-- 
1.8.3.1

[PATCH 0/5] Add ASoC support for AMD APUs [v3]

[Alex Deucher]

This patch set implements support for i2s audio and new AMD GPUs.
The i2s codec is fed by a DMA engine on the GPU.  To handle this
we create mfd cells which we hang the i2s codec and DMA engine on.
Because of this, this patch set covers two subsystems: drm and alsa.
The drm patches add support for the ACP hw block which provides the
DMA engine for the i2s codec.  The alsa patches add the ASoC driver
for the i2s codec.  Since the alsa changes depend on the drm changes
in this patch set as well as some other drm changes queued for 4.3,
I'd like to take the alsa patches in via the drm tree.

V2 changes:
- Use the MFD subsystem rather than adding our own bus
- Squash all sub-feature patches together
- fix comments mentioned in previous review

V3 changes:
- Update the designware driver to handle slave mode, amd specific
  features
- Use the designware driver directly for i2s
- Move the DMA handling from the GPU driver into the AMD ASoC
  driver
- Change the license on the ASoC driver to GPL

Patch 4 adds the register headers for the ACP block which is a
pretty big patch so I've excluded it from email.  The entire patch
set can be viewed here:
http://cgit.freedesktop.org/~agd5f/linux/log/?h=acp-upstream3

Thanks,

Alex

Maruthi Bayyavarapu (1):
  drm/amd: add ACP driver support

Maruthi Srinivas Bayyavarapu (4):
  ASoC : dwc : support dw i2s in slave mode
  ASoC : dwc : support dw i2s in AMD platform
  ASoC : AMD : add ACP 2.2 register headers
  ASoC: AMD: add AMD ASoC ACP-I2S driver

 drivers/gpu/drm/Kconfig  |2 +
 drivers/gpu/drm/amd/acp/Kconfig  |   10 +
 drivers/gpu/drm/amd/acp/Makefile |9 +
 drivers/gpu/drm/amd/acp/acp_hw.c |  127 ++
 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h |   49 +
 drivers/gpu/drm/amd/amdgpu/Makefile  |   13 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |   12 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c  |  269 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h  |   41 +
 drivers/gpu/drm/amd/amdgpu/vi.c  |   12 +
 drivers/gpu/drm/amd/include/amd_shared.h |1 +
 include/linux/mfd/amd_acp.h  |   43 +
 include/sound/designware_i2s.h   |4 +
 sound/soc/Kconfig|1 +
 sound/soc/Makefile   |1 +
 sound/soc/amd/Kconfig|4 +
 sound/soc/amd/Makefile   |3 +
 sound/soc/amd/acp-pcm-dma.c  |  518 ++
 sound/soc/amd/acp.c  |  736 +
 sound/soc/amd/acp.h  |  147 ++
 sound/soc/amd/include/acp_2_2_d.h|  609 +++
 sound/soc/amd/include/acp_2_2_enum.h | 1068 
 sound/soc/amd/include/acp_2_2_sh_mask.h  | 2292 ++
 sound/soc/dwc/designware_i2s.c   |  270 +--
 24 files changed, 6141 insertions(+), 100 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/acp/Kconfig
 create mode 100644 drivers/gpu/drm/amd/acp/Makefile
 create mode 100644 drivers/gpu/drm/amd/acp/acp_hw.c
 create mode 100644 drivers/gpu/drm/amd/acp/include/acp_gfx_if.h
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.h
 create mode 100644 include/linux/mfd/amd_acp.h
 create mode 100644 sound/soc/amd/Kconfig
 create mode 100644 sound/soc/amd/Makefile
 create mode 100644 sound/soc/amd/acp-pcm-dma.c
 create mode 100644 sound/soc/amd/acp.c
 create mode 100644 sound/soc/amd/acp.h
 create mode 100644 sound/soc/amd/include/acp_2_2_d.h
 create mode 100644 sound/soc/amd/include/acp_2_2_enum.h
 create mode 100644 sound/soc/amd/include/acp_2_2_sh_mask.h

-- 
1.8.3.1

[PATCH 02/23] drm: Add structure for querying palette color capabilities

[Sharma, Shashank]

This would be an interface/design change, to change from one blob of correction 
property, to split into multiple query properties for palette_before_blob and 
palette_after_blob. 
Please let me know if this is really required ?

Regards
Shashank
-Original Message-
From: Smith, Gary K 
Sent: Wednesday, September 23, 2015 3:17 PM
To: Sharma, Shashank; Daniel Vetter; Roper, Matthew D
Cc: Matheson, Annie J; Bradford, Robert; Palleti, Avinash Reddy; intel-gfx at 
lists.freedesktop.org; dri-devel at lists.freedesktop.org; Mukherjee, Indranil; 
Bish, Jim; kausalmalladi at gmail.com; Vetter, Daniel
Subject: RE: [PATCH 02/23] drm: Add structure for querying palette color 
capabilities

Given that its only one word of info per LUT, I'm OK with it being two separate 
properties. 
I believe it was much more complex previously with a lot more info per LUT, 
which is probably why I preferred a blob.

Thanks
Gary

-Original Message-
From: Sharma, Shashank
Sent: Wednesday, September 23, 2015 9:10 AM
To: Daniel Vetter; Roper, Matthew D
Cc: Matheson, Annie J; Bradford, Robert; Palleti, Avinash Reddy; intel-gfx at 
lists.freedesktop.org; dri-devel at lists.freedesktop.org; Mukherjee, Indranil; 
Bish, Jim; Smith, Gary K; kausalmalladi at gmail.com; Vetter, Daniel
Subject: Re: [PATCH 02/23] drm: Add structure for querying palette color 
capabilities

Hi Matt, Daniel
Addressing the review comments from both of you here.

Regards
Shashank

On 9/22/2015 6:32 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 10:51:31AM -0700, Matt Roper wrote:
>> On Wed, Sep 16, 2015 at 11:06:59PM +0530, Shashank Sharma wrote:
>>> From: Kausal Malladi 
>>>
>>> The DRM color management framework is targeting various hardware 
>>> platforms and drivers. Different platforms can have different color 
>>> correction and enhancement capabilities.
>>>
>>> A commom user space application can query these capabilities using 
>>> the DRM property interface. Each driver can fill this property with 
>>> its platform's palette color capabilities.
>>>
>>> This patch adds new structure in DRM layer for querying palette 
>>> color capabilities. This structure will be used by all user space 
>>> agents to configure appropriate color configurations.
>>>
>>> Signed-off-by: Shashank Sharma 
>>> Signed-off-by: Kausal Malladi 
>>
>> I think you provided an explanation on a previous code review cycle, 
>> but I forget the details now...what's the benefit to using a blob for 
>> caps rather than having these be individual properties?  Individual 
>> properties seems more natural to me, but I think you had a 
>> justification for blobbing them together; that reasoning would be 
>> good to include in the commit message.
>
> Yeah I'm leaning slightly towards individual props too, that would 
> give us a bit more freedom with placing them (e.g. if someone comes up 
> with funky hw where before_ctm and ctm are per-plane and after_ctm is 
> on the crtc, with only some planes support the before_ctm gamma table).
This was the part where we spent most of the time during the design review, and 
the reason we came up for this was:
- This is a read only property, which userspace would like to read only once, 
and cache the information. It was also Gary's opinion to keep this as single 
blob for all.
- Making individual property needs more information to be provided to user 
space.
- This is a blob only for pipe level capabilities, the plane level blob will be 
separate from this.
- We can handle this HW also, by loading proper plane and pipe level capability 
blob. This is more convenient to have all the capabilities together at the same 
place, than keep on querying the same.
>
> Also if you do per-prop properties instead of the blob you can drop 
> the version/reserved fields, since properties are inheritedly designed 
> to be extendible. So no need to revision them again (it only leads to 
> more code that might break).
> -Daniel
>
We are anyways planning to drop the version, as per Ville's comment.
- Shashank
>>
>>
>> Matt
>>
>>> ---
>>>   include/uapi/drm/drm.h | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 
>>> 3801584..e3c642f 100644
>>> --- a/include/uapi/drm/drm.h
>>> +++ b/include/uapi/drm/drm.h
>>> @@ -829,6 +829,17 @@ struct drm_event_vblank {
>>> __u32 reserved;
>>>   };
>>>
>>> +struct drm_palette_caps {
>>> +   /* Structure version. Should be 1 currently */
>>> +   __u32 version;
>>> +   /* For padding and future use */
>>> +   __u32 reserved;
>>> +   /* This may be 0 if not supported. e.g. plane palette or VLV pipe */
>>> +   __u32 num_samples_before_ctm;
>>> +   /* This will be non-zero for pipe. May be zero for planes on some HW */
>>> +   __u32 num_samples_after_ctm;
>>> +};
>>> +
>>>   /* typedef area */
>>>   #ifndef __KERNEL__
>>>   typedef struct drm_clip_rect drm_clip_rect_t;
>>> --
>>> 1.9.1
>>>
>>
>> --
>> Matt Roper
>> Graphics Software Engineer
>> 

[PATCH v3 3/3] modetest: add atomic page flip support

[Hyungwon Hwang]

This patch adds support for atomic page flip. User can specify -V option
with the plane id for testing atomic page flipping.

Signed-off-by: Hyungwon Hwang 
---
 tests/modetest/modetest.c | 195 --
 1 file changed, 187 insertions(+), 8 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index bc5a227..418acaa 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -747,6 +747,10 @@ struct pipe_arg {
struct timeval start;

int swap_count;
+
+   /* for atomic modeset */
+   uint32_t plane_id;
+   uint32_t fb_obj_id;
 };

 struct plane_arg {
@@ -1477,7 +1481,7 @@ static int parse_property(struct property_arg *p, const 
char *arg)

 static void usage(char *name)
 {
-   fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
+   fprintf(stderr, "usage: %s [-acDdefMPpsCvVw]\n", name);
fprintf(stderr, "\tA: supported in atomic modeset\n");
fprintf(stderr, "\tL: supported in legacy modeset\n");

@@ -1492,6 +1496,7 @@ static void usage(char *name)

fprintf(stderr, "\n Atomic Test options: [A]\n\n");
fprintf(stderr, "\t-a\tuse atomic modeset\n");
+   fprintf(stderr, "\t-V \ttest vsynced page 
flipping\n");

fprintf(stderr, "\n Legacy test options: [L]\n\n");
fprintf(stderr, "\t-P 
:x[++][*][@]\tset a plane\n");
@@ -1641,7 +1646,8 @@ static int allocate_fb(int fd, drmModeAtomicReqPtr req, 
struct resources *res,

 static int allocate_fbs(struct device *dev, drmModeAtomicReqPtr req,
struct resources *res, struct property_arg *prop_args,
-   unsigned int prop_count, struct bo **bo, uint32_t 
*fb_id)
+   unsigned int prop_count, struct bo **bo, uint32_t 
*fb_id,
+   uint32_t flip_plane_id)
 {
uint32_t plane_id, fb_obj_id, pixel_format;
uint64_t width, height;
@@ -1652,6 +1658,9 @@ static int allocate_fbs(struct device *dev, 
drmModeAtomicReqPtr req,
if (!is_obj_id_in_prop_args(prop_args, prop_count, plane_id))
continue;

+   if (flip_plane_id == plane_id)
+   dev->mode.fb_id = fb_id[i];
+
fb_obj_id = get_plane_prop_id(res, plane_id, "FB_ID");
if (!fb_obj_id) {
fprintf(stderr, "plane(%u) does not exist\n", plane_id);
@@ -1714,8 +1723,162 @@ static void deallocate_fbs(int fd, int num_planes, 
uint32_t *fb_id, struct bo **
}
 }

+static void atomic_page_flip_handler(int fd, unsigned int frame, unsigned int 
sec,
+   unsigned int usec, void *data)
+{
+   static drmModeAtomicReqPtr req = NULL;
+   unsigned int new_fb_id;
+   struct timeval end;
+   struct pipe_arg *pipe;
+   double t;
+   uint32_t flags = 0;
+   int ret;
+
+   pipe = (struct pipe_arg *)(unsigned long)data;
+
+   req = drmModeAtomicAlloc();
+   if (!req) {
+   fprintf(stderr, "failed to allocate drmModeAtomicReqPtr\n");
+   return;
+   }
+
+   if (pipe->current_fb_id == pipe->fb_id[0])
+   new_fb_id = pipe->fb_id[1];
+   else
+   new_fb_id = pipe->fb_id[0];
+
+   pipe->current_fb_id = new_fb_id;
+   pipe->swap_count++;
+
+   ret = drmModeAtomicAddProperty(req, pipe->plane_id, pipe->fb_obj_id, 
new_fb_id);
+   if (ret < 0) {
+   fprintf(stderr, "failed to add atomic property in pageflip 
handler\n");
+   drmModeAtomicFree(req);
+   return;
+   }
+
+   flags = DRM_MODE_PAGE_FLIP_EVENT;
+   ret = drmModeAtomicCommit(fd, req, flags, pipe);
+   if (ret < 0) {
+   fprintf(stderr, "failed to commit in pageflip handler\n");
+   drmModeAtomicFree(req);
+   return;
+   }
+
+   if (pipe->swap_count == 60) {
+   gettimeofday(, NULL);
+   t = end.tv_sec + end.tv_usec * 1e-6 -
+   (pipe->start.tv_sec + pipe->start.tv_usec * 1e-6);
+   fprintf(stderr, "freq: %.02fHz\n", pipe->swap_count / t);
+   pipe->swap_count = 0;
+   pipe->start = end;
+   }
+
+   drmModeAtomicFree(req);
+}
+
+static void atomic_test_page_flip(struct device *dev, drmModeAtomicReqPtr req,
+   struct resources *res, struct property_arg *prop_args,
+   unsigned int prop_count, unsigned int flip_plane_id)
+{
+   struct bo *other_bo;
+   unsigned int other_fb_id;
+   struct pipe_arg pipe;
+   drmEventContext evctx;
+   uint32_t flags = 0, fb_obj_id = 0, pixel_format;
+   uint64_t width, height;
+   int ret;
+
+   fb_obj_id = get_plane_prop_id(res, flip_plane_id, "FB_ID");
+   if (!fb_obj_id) {
+   fprintf(stderr, "plane(%u) does not exist\n", flip_plane_id);
+   return;
+   

[PATCH v3 2/3] modetest: add atomic modeset support

[Hyungwon Hwang]

This patch adds support for atomic modeset. Using -a option, user can
make modeset to use DRM_IOCTL_MODE_ATOMIC instead of legacy IOCTLs.
Also, by using -w option, user can set the property as before.

Signed-off-by: Hyungwon Hwang 
---
 tests/modetest/modetest.c | 273 --
 1 file changed, 265 insertions(+), 8 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 08ecf58..bc5a227 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1477,25 +1477,32 @@ static int parse_property(struct property_arg *p, const 
char *arg)

 static void usage(char *name)
 {
-   fprintf(stderr, "usage: %s [-cDdefMPpsCvw]\n", name);
+   fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
+   fprintf(stderr, "\tA: supported in atomic modeset\n");
+   fprintf(stderr, "\tL: supported in legacy modeset\n");

-   fprintf(stderr, "\n Query options:\n\n");
+   fprintf(stderr, "\n Query options: [AL]\n\n");
fprintf(stderr, "\t-c\tlist connectors\n");
fprintf(stderr, "\t-e\tlist encoders\n");
fprintf(stderr, "\t-f\tlist framebuffers\n");
fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n");

-   fprintf(stderr, "\n Test options:\n\n");
+   fprintf(stderr, "\n Common Test options: [AL]\n\n");
+   fprintf(stderr, "\t-w ::\tset property\n");
+
+   fprintf(stderr, "\n Atomic Test options: [A]\n\n");
+   fprintf(stderr, "\t-a\tuse atomic modeset\n");
+
+   fprintf(stderr, "\n Legacy test options: [L]\n\n");
fprintf(stderr, "\t-P 
:x[++][*][@]\tset a plane\n");
fprintf(stderr, "\t-s 
[,][@]:[-][@]\tset 
a mode\n");
fprintf(stderr, "\t-C\ttest hw cursor\n");
fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
-   fprintf(stderr, "\t-w ::\tset property\n");

fprintf(stderr, "\n Generic options:\n\n");
-   fprintf(stderr, "\t-d\tdrop master after mode set\n");
-   fprintf(stderr, "\t-M module\tuse the given driver\n");
-   fprintf(stderr, "\t-D device\tuse the given device\n");
+   fprintf(stderr, "\t-d\tdrop master after mode set [L]\n");
+   fprintf(stderr, "\t-M module\tuse the given driver [AL]\n");
+   fprintf(stderr, "\t-D device\tuse the given device [AL]\n");

fprintf(stderr, "\n\tDefault is to dump all info.\n");
exit(0);
@@ -1554,7 +1561,224 @@ static int pipe_resolve_connectors(struct device *dev, 
struct pipe_arg *pipe)
return 0;
 }

-static char optstr[] = "cdD:efM:P:ps:Cvw:";
+static bool is_obj_id_in_prop_args(struct property_arg *prop_args,
+   unsigned int prop_count, uint32_t obj_id)
+{
+   unsigned int i;
+
+   for (i = 0; i < prop_count; i++)
+   if (obj_id == prop_args[i].obj_id)
+   return true;
+
+   return false;
+}
+
+static int get_value_in_prop_args(struct property_arg *prop_args,
+   unsigned int prop_count, uint32_t obj_id,
+   const char *name, uint64_t *out)
+{
+   unsigned int i;
+
+   for (i = 0; i < prop_count; i++) {
+   if (prop_args[i].obj_id == obj_id &&
+   !strcmp(prop_args[i].name, name)) {
+   *out = prop_args[i].value;
+   return 0;
+   }
+   }
+
+   return -1;
+}
+
+static uint32_t get_plane_prop_id(struct resources *res, uint32_t obj_id,
+   const char *name)
+{
+   drmModePropertyRes *props_info;
+   struct plane *plane;
+   unsigned int i, j;
+
+   for (i = 0; i < res->plane_res->count_planes; i++) {
+   plane = >planes[i];
+   if (plane->plane->plane_id != obj_id)
+   continue;
+
+   for (j = 0; j < plane->props->count_props; j++) {
+   props_info = plane->props_info[j];
+   if (!strcmp(props_info->name, name))
+   return props_info->prop_id;
+   }
+   }
+
+   return 0;
+}
+
+static int allocate_fb(int fd, drmModeAtomicReqPtr req, struct resources *res,
+   uint32_t width, uint32_t height, uint32_t pixel_format,
+   int pattern, struct bo **bo, uint32_t *fb_id)
+{
+   uint32_t handles[4] = {0}, pitches[4] = {0}, offsets[4] = {0};
+   int ret;
+
+   *bo = bo_create(fd, pixel_format, width, height,
+   handles, pitches, offsets, pattern);
+   if (*bo == NULL) {
+   fprintf(stderr, "failed to create bo (%ux%u): %s\n",
+   width, height, strerror(errno));
+   return -1;
+   }
+
+   ret = drmModeAddFB2(fd, width, height, pixel_format,
+   handles, pitches, offsets, fb_id, 0);
+   if (ret) {
+   fprintf(stderr, "failed to add fb (%ux%u): %s\n",
+ 

[PATCH v3 1/3] modetest: introduce get_prop_info() for getting property id and type

[Hyungwon Hwang]

Modetest gets the property name from user to set it. So the name must be
converted to its id. Until now, this is done in the set_property(). But to
support atomic modeset in modetest, this logic should be separated from the
fuction, because atomic modeset and legacy modeset use different IOCTLs.

Signed-off-by: Hyungwon Hwang 
---
 tests/modetest/modetest.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 3b01918..08ecf58 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -894,12 +894,11 @@ struct property_arg {
uint64_t value;
 };

-static void set_property(struct device *dev, struct property_arg *p)
+static int get_prop_info(struct resources *resources, struct property_arg *p,
+   const char *obj_type)
 {
drmModeObjectProperties *props = NULL;
drmModePropertyRes **props_info = NULL;
-   const char *obj_type;
-   int ret;
int i;

p->obj_type = 0;
@@ -918,21 +917,21 @@ static void set_property(struct device *dev, struct 
property_arg *p)
}   
\
} while(0)  
\

-   find_object(dev->resources, res, crtc, CRTC);
+   find_object(resources, res, crtc, CRTC);
if (p->obj_type == 0)
-   find_object(dev->resources, res, connector, CONNECTOR);
+   find_object(resources, res, connector, CONNECTOR);
if (p->obj_type == 0)
-   find_object(dev->resources, plane_res, plane, PLANE);
+   find_object(resources, plane_res, plane, PLANE);
if (p->obj_type == 0) {
fprintf(stderr, "Object %i not found, can't set property\n",
p->obj_id);
-   return;
+   return -1;
}

if (!props) {
fprintf(stderr, "%s %i has no properties\n",
obj_type, p->obj_id);
-   return;
+   return -1;
}

for (i = 0; i < (int)props->count_props; ++i) {
@@ -945,11 +944,23 @@ static void set_property(struct device *dev, struct 
property_arg *p)
if (i == (int)props->count_props) {
fprintf(stderr, "%s %i has no %s property\n",
obj_type, p->obj_id, p->name);
-   return;
+   return -1;
}

p->prop_id = props->props[i];

+   return 0;
+}
+
+static void set_property(struct device *dev, struct property_arg *p)
+{
+   int ret;
+   const char *obj_type = NULL;
+
+   ret = get_prop_info(dev->resources, p, obj_type);
+   if (ret < 0)
+   return;
+
ret = drmModeObjectSetProperty(dev->fd, p->obj_id, p->obj_type,
   p->prop_id, p->value);
if (ret < 0)
--
2.4.3

[PATCH 3/3] modetest: add atomic page flip support

[Hyungwon Hwang]

Dear Emil,

On Wed, 02 Sep 2015 01:43:41 +0100
Emil Velikov  wrote:

> On 26 August 2015 at 07:21, Hyungwon Hwang 
> wrote:
> > This patch adds support for atomic page flip. User can specify -V
> > option with the plane id for testing atomic page flipping.
> > ---
> >  tests/modetest/modetest.c | 153
> > -- 1 file changed, 149
> > insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 753a559..9bffa98 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -719,6 +719,10 @@ struct pipe_arg {
> > struct timeval start;
> >
> > int swap_count;
> > +
> > +   /* for atomic modeset */
> > +   uint32_t plane_id;
> > +   uint32_t fb_obj_id;
> >  };
> >
> >  struct plane_arg {
> > @@ -1444,7 +1448,7 @@ static int parse_property(struct property_arg
> > *p, const char *arg)
> >
> >  static void usage(char *name)
> >  {
> > -   fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
> > +   fprintf(stderr, "usage: %s [-acDdefMPpsCvVw]\n", name);
> > fprintf(stderr, "\tA: supported in atomic modeset\n");
> > fprintf(stderr, "\tL: supported in legacy modeset\n");
> >
> > @@ -1459,6 +1463,7 @@ static void usage(char *name)
> >
> > fprintf(stderr, "\n Atomic Test options: [A]\n\n");
> > fprintf(stderr, "\t-a\tuse atomic modeset\n");
> > +   fprintf(stderr, "\t-V \ttest vsynced
> > page flipping\n");
> >
> > fprintf(stderr, "\n Legacy test options: [L]\n\n");
> > fprintf(stderr, "\t-P
> > :x[++][*][@]\tset a plane\n");
> > @@ -1627,7 +1632,138 @@ static void atomic_modeset(struct device
> > *dev, drmModeAtomicReqPtr req, drmModeAtomicCommit(dev->fd, req,
> > flags, NULL); }
> >
> > -static char optstr[] = "acdD:efM:P:ps:Cvw:";
> > +static void
> > +atomic_page_flip_handler(int fd, unsigned int frame,
> > +   unsigned int sec, unsigned int usec, void *data)
> > +{
> > +   static drmModeAtomicReqPtr req = NULL;
> > +   unsigned int new_fb_id;
> > +   struct timeval end;
> > +   struct pipe_arg *pipe;
> > +   double t;
> > +   uint32_t flags = 0;
> > +
> > +   pipe = (struct pipe_arg *)(unsigned long)data;
> > +
> > +   if (pipe->current_fb_id == pipe->fb_id[0])
> > +   new_fb_id = pipe->fb_id[1];
> > +   else
> > +   new_fb_id = pipe->fb_id[0];
> > +
> > +   pipe->current_fb_id = new_fb_id;
> > +   pipe->swap_count++;
> > +
> > +   req = drmModeAtomicAlloc();
> > +
> > +   drmModeAtomicAddProperty(req, pipe->plane_id,
> > pipe->fb_obj_id, new_fb_id); +
> We'll crash badly if req is NULL here. I guess we can smoke test the
> API, but that is no excuse for the missing null check.
> 
> > +   flags = DRM_MODE_PAGE_FLIP_EVENT;
> > +   drmModeAtomicCommit(fd, req, flags, pipe);
> > +
> > +   if (pipe->swap_count == 60) {
> > +   gettimeofday(, NULL);
> > +   t = end.tv_sec + end.tv_usec * 1e-6 -
> > +   (pipe->start.tv_sec + pipe->start.tv_usec *
> > 1e-6);
> > +   fprintf(stderr, "freq: %.02fHz\n",
> > pipe->swap_count / t);
> > +   pipe->swap_count = 0;
> > +   pipe->start = end;
> > +   }
> > +
> > +   drmModeAtomicFree(req);
> > +}
> > +
> > +static int atomic_test_page_flip(struct device *dev,
> > drmModeAtomicReqPtr req,
> > +   struct resources *res, struct property_arg
> > *prop_args,
> > +   unsigned int prop_count,unsigned int
> > plane_id) +{
> > +   struct bo *other_bo;
> > +   unsigned int other_fb_id;
> > +   struct pipe_arg *pipe = NULL;
> > +   drmEventContext evctx;
> > +   uint32_t flags = 0, fb_obj_id = 0, pixel_format;
> > +   uint64_t width, height;
> > +   int ret;
> > +
> > +   if (!is_obj_id_in_prop_args(prop_args, prop_count,
> > plane_id))
> > +   return -1;
> > +
> > +   fb_obj_id = get_atomic_plane_prop_id(res, plane_id,
> > "FB_ID");
> > +   if (!fb_obj_id)
> > +   return -1;
> > +
> > +   width = get_value_in_prop_args(prop_args, prop_count,
> > plane_id,
> > +   "SRC_W");
> > +   height = get_value_in_prop_args(prop_args, prop_count,
> > plane_id,
> > +   "SRC_H");
> Here and in the previous patch you assume that always succeeds.
> Perhaps one should check it otherwise all sort of crazy stuff will
> happen in allocate_fb() ?
> 
> > +   pixel_format = DRM_FORMAT_XRGB;
> > +
> For the future we might consider making this user configurable,
> although as is it looks like a good default.
> 
> > +   ret = allocate_fb(dev, req, res, width, height,
> > pixel_format,
> > +   PATTERN_TILES, _bo, _fb_id);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   ret = drmModeAtomicAddProperty(req, plane_id, 

4.3-rc2 on radeon: new backtraces during resume

[Alex Deucher]

On Wed, Sep 23, 2015 at 7:51 AM, Pavel Machek  wrote:
> HI!
>
>
> I suspended T40p by mistake, and I got some lovely backtraces as a
> result:
>
> Any ideas?

Please see this thread:
https://lkml.org/lkml/2015/9/23/361

Alex

>
> Pavel
>
> [0.00] Initializing cgroup subsys cpu
> [0.00] Linux version 4.3.0-rc2+ (pavel at hobit) (gcc version 4.9.2 
> (Debian 4.9.2-10) ) #111 SMP Wed Sep 23 13:29:04 CEST 2015
> ...
> [  855.275406] PM: Syncing filesystems ... done.
> [  855.745355] PM: Preparing system for sleep (mem)
> [  855.830116] Freezing user space processes ... (elapsed 0.010 seconds) done.
> [  855.841061] Freezing remaining freezable tasks ... (elapsed 0.003 seconds) 
> done.
> [  855.844739] PM: Suspending system (mem)
> [  855.844809] Suspending console(s) (use no_console_suspend to debug)
> [  856.054406] parport_pc 00:07: disabled
> [  856.055141] serial 00:06: disabled
> [  856.056458] serial 00:06: System wakeup disabled by ACPI
> [  856.058401] sd 0:0:0:0: [sda] Synchronizing SCSI cache
> [  856.059365] [ cut here ]
> [  856.059381] WARNING: CPU: 0 PID: 6609 at include/drm/drm_crtc.h:1577 
> drm_helper_choose_encoder_dpms+0x82/0x90()
> [  856.059384] Modules linked in:
> [  856.059391] CPU: 0 PID: 6609 Comm: kworker/u2:7 Not tainted 4.3.0-rc2+ #111
> [  856.059393] Hardware name: IBM 2373G3U/2373G3U, BIOS 1RETDNWW (3.19 ) 
> 10/13/2005
> [  856.059402] Workqueue: events_unbound async_run_entry_fn
> [  856.059416]    de099d94 c42b40a8  de099db0 
> c403ecdb 0629
> [  856.059427]  c438fd82 f6acd800 f585ee00 0003 de099dc0 c403ed7f 
> 0009 
> [  856.059437]  de099dd0 c438fd82 f585ee00 f600 de099df0 c4390132 
> f6acd800 
> [  856.059439] Call Trace:
> [  856.059450]  [] dump_stack+0x41/0x59
> [  856.059457]  [] warn_slowpath_common+0x6b/0xa0
> [  856.059462]  [] ? drm_helper_choose_encoder_dpms+0x82/0x90
> [  856.059467]  [] warn_slowpath_null+0xf/0x20
> [  856.059472]  [] drm_helper_choose_encoder_dpms+0x82/0x90
> [  856.059477]  [] drm_helper_connector_dpms+0x32/0xf0
> [  856.059485]  [] radeon_suspend_kms+0x64/0x350
> [  856.059491]  [] radeon_pmops_suspend+0x18/0x20
> [  856.059497]  [] pci_pm_suspend+0x5d/0x130
> [  856.059502]  [] ? pci_pm_freeze+0xc0/0xc0
> [  856.059509]  [] dpm_run_callback+0x30/0x70
> [  856.059514]  [] __device_suspend+0xd2/0x290
> [  856.059522]  [] ? __lock_acquire.isra.24+0x3b1/0xca0
> [  856.059526]  [] async_suspend+0x17/0x90
> [  856.059531]  [] async_run_entry_fn+0x4f/0x140
> [  856.059537]  [] process_one_work+0x15f/0x3a0
> [  856.059542]  [] ? process_one_work+0x15f/0x3a0
> [  856.059546]  [] ? process_one_work+0x109/0x3a0
> [  856.059551]  [] worker_thread+0x39/0x430
> [  856.059556]  [] ? process_one_work+0x3a0/0x3a0
> [  856.059562]  [] kthread+0xae/0xd0
> [  856.059572]  [] ret_from_kernel_thread+0x21/0x30
> [  856.059577]  [] ? kthread_create_on_node+0x170/0x170
> [  856.059581] ---[ end trace c98a514a441969f1 ]---
> [  856.079439] [ cut here ]
> [  856.079446] WARNING: CPU: 0 PID: 6609 at include/drm/drm_crtc.h:1577 
> drm_helper_choose_crtc_dpms+0x82/0x90()
> [  856.079449] Modules linked in:
> [  856.079454] CPU: 0 PID: 6609 Comm: kworker/u2:7 Tainted: GW   
> 4.3.0-rc2+ #111
> [  856.079457] Hardware name: IBM 2373G3U/2373G3U, BIOS 1RETDNWW (3.19 ) 
> 10/13/2005
> [  856.079462] Workqueue: events_unbound async_run_entry_fn
> [  856.079473]    de099d94 c42b40a8  de099db0 
> c403ecdb 0629
> [  856.079484]  c438fe12 f600 f6acd800 0003 de099dc0 c403ed7f 
> 0009 
> [  856.079495]  de099dd0 c438fe12 c43e24c0 f600 de099df0 c43901d6 
> f6acd800 
> [  856.079496] Call Trace:
> [  856.079503]  [] dump_stack+0x41/0x59
> [  856.079508]  [] warn_slowpath_common+0x6b/0xa0
> [  856.079512]  [] ? drm_helper_choose_crtc_dpms+0x82/0x90
> [  856.079517]  [] warn_slowpath_null+0xf/0x20
> [  856.079522]  [] drm_helper_choose_crtc_dpms+0x82/0x90
> [  856.079529]  [] ? radeon_crtc_mode_fixup+0x10/0x10
> [  856.079534]  [] drm_helper_connector_dpms+0xd6/0xf0
> [  856.079539]  [] radeon_suspend_kms+0x64/0x350
> [  856.079544]  [] radeon_pmops_suspend+0x18/0x20
> [  856.079549]  [] pci_pm_suspend+0x5d/0x130
> [  856.079554]  [] ? pci_pm_freeze+0xc0/0xc0
> [  856.079558]  [] dpm_run_callback+0x30/0x70
> [  856.079563]  [] __device_suspend+0xd2/0x290
> [  856.079568]  [] ? __lock_acquire.isra.24+0x3b1/0xca0
> [  856.079573]  [] async_suspend+0x17/0x90
> [  856.079578]  [] async_run_entry_fn+0x4f/0x140
> [  856.079583]  [] process_one_work+0x15f/0x3a0
> [  856.079587]  [] ? process_one_work+0x15f/0x3a0
> [  856.079592]  [] ? process_one_work+0x109/0x3a0
> [  856.079597]  [] worker_thread+0x39/0x430
> [  856.079601]  [] ? process_one_work+0x3a0/0x3a0
> [  856.079606]  [] kthread+0xae/0xd0
> [  856.079613]  [] 

[regression] [git pull] drm for 4.3

[Lankhorst, Maarten]

Hey,

Dave Jones schreef op di 22-09-2015 om 21:49 [-0400]:
> On Tue, Sep 22, 2015 at 09:15:58AM -0700, Matt Roper wrote:
>  > On Tue, Sep 22, 2015 at 05:13:55PM +0200, Daniel Vetter wrote:
>  > > On Tue, Sep 22, 2015 at 08:00:17AM -0700, Jesse Barnes wrote:
>  > > > Cc'ing Maarten and Matt; I'm guessing this may be related to one of
>  > > > their recent patches.
>  > 
>  > Sounds like this showed up before my recent work, but I think I might
>  > have seen similar problems while working on atomic watermarks; the
>  > issues I was seeing were because the initial hardware readout could
>  > leave primary->visible set to true even when the CRTC was off.  My
>  > series (which is still under development) contains this patch to fix
>  > that:
>  > 
>  > http://patchwork.freedesktop.org/patch/59564/
>  > 
>  > Does applying that help with the problems reported here?
> 
> No difference at all for me.
Looks like a (reopened) dup of 91952?

Can you apply "[PATCH] drm/i915: Add primary plane to mask if it's
visible", and get me the results?

~Maarten
-
Intel International B.V.
Registered in The Netherlands under number 34098535
Statutory seat: Haarlemmermeer
Registered address: Capronilaan 37, 1119NG Schiphol-Rijk

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()

[Borislav Petkov]

On Wed, Sep 23, 2015 at 09:25:23AM +0200, Daniel Vetter wrote:
> Strange thing is that I've tested this on a radeon over here and I don't
> see this backtrace ... wut. Below diff should appease the backtraces at
> least.

Doesn't look like it.

This is what it says when suspending:


[   42.962275] hib.sh (3269): drop_caches: 3
[   42.967671] PM: Hibernation mode set to 'shutdown'
[   42.979329] PM: Syncing filesystems ... done.
[   42.993401] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   43.003632] PM: Marking nosave pages: [mem 0x-0x0fff]
[   43.009840] PM: Marking nosave pages: [mem 0x0009e000-0x000f]
[   43.015991] PM: Marking nosave pages: [mem 0xba9b8000-0xbca4dfff]
[   43.022241] PM: Marking nosave pages: [mem 0xbca4f000-0xbcc54fff]
[   43.028357] PM: Marking nosave pages: [mem 0xbd083000-0xbd7f3fff]
[   43.034500] PM: Marking nosave pages: [mem 0xbd80-0x10fff]
[   43.041371] PM: Basic memory bitmaps created
[   43.045656] PM: Preallocating image memory... done (allocated 128867 pages)
[   43.346216] PM: Allocated 515468 kbytes in 0.29 seconds (1777.47 MB/s)
[   43.352759] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[   43.366104] [ cut here ]
[   43.370746] WARNING: CPU: 4 PID: 55 at include/drm/drm_crtc.h:1577 
drm_helper_choose_encoder_dpms+0x88/0x90()
[   43.380681] Modules linked in: binfmt_misc ipv6 vfat fat fuse dm_crypt 
dm_mod kvm_amd kvm crc32_pclmul aesni_intel ae
s_x86_64 lrw gf128mul glue_helper ablk_helper cryptd amd64_edac_mod k10temp 
fam15h_power edac_core amdkfd amd_iommu_v2 r
adeon acpi_cpufreq
[   43.403916] CPU: 4 PID: 55 Comm: kworker/u16:2 Not tainted 4.3.0-rc2+ #3
[   43.410633] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
[   43.420567] Workqueue: events_unbound async_run_entry_fn
[   43.425919]  8194ff67 88042a223b60 812c758a 

[   43.433424]  88042a223b98 810534c1 880429eca000 
880429fe9200
[   43.440959]  880429de1000  819571c3 
88042a223ba8
[   43.448461] Call Trace:
[   43.450929]  [] dump_stack+0x4e/0x84
[   43.456094]  [] warn_slowpath_common+0x91/0xd0
[   43.462126]  [] warn_slowpath_null+0x1a/0x20
[   43.467983]  [] drm_helper_choose_encoder_dpms+0x88/0x90
[   43.474884]  [] drm_helper_connector_dpms+0x56/0x110
[   43.481463]  [] radeon_suspend_kms+0x6b/0x380 [radeon]
[   43.488196]  [] ? _raw_spin_unlock_irqrestore+0x4b/0x80
[   43.495020]  [] ? pci_pm_poweroff+0x100/0x100
[   43.500976]  [] radeon_pmops_freeze+0x1c/0x20 [radeon]
[   43.507730]  [] pci_pm_freeze+0x6a/0x100
[   43.513241]  [] ? pci_pm_poweroff+0x100/0x100
[   43.519187]  [] dpm_run_callback+0x77/0x2a0
[   43.524959]  [] __device_suspend+0x104/0x2c0
[   43.530818]  [] async_suspend+0x1f/0xa0
[   43.536242]  [] async_run_entry_fn+0x46/0xf0
[   43.542127]  [] process_one_work+0x1f8/0x640
[   43.547984]  [] ? process_one_work+0x154/0x640
[   43.554019]  [] worker_thread+0x4b/0x440
[   43.559530]  [] ? preempt_count_sub+0xb3/0x110
[   43.565562]  [] ? process_one_work+0x640/0x640
[   43.571603]  [] kthread+0xf6/0x110
[   43.576597]  [] ? kthread_create_on_node+0x1f0/0x1f0
[   43.583152]  [] ret_from_fork+0x3f/0x70
[   43.588574]  [] ? kthread_create_on_node+0x1f0/0x1f0
[   43.595170] ---[ end trace aab225b93a6f1dcc ]---
[   43.595235] [ cut here ]
[   43.595240] WARNING: CPU: 5 PID: 55 at include/drm/drm_crtc.h:1577 
drm_helper_choose_crtc_dpms+0x91/0xa0()
[   43.595267] Modules linked in: binfmt_misc ipv6 vfat fat fuse dm_crypt 
dm_mod kvm_amd kvm crc32_pclmul aesni_intel aes_x86_64 lrw gf128mul glue_helper 
ablk_helper cryptd amd64_edac_mod k10temp fam15h_power edac_core amdkfd 
amd_iommu_v2 radeon acpi_cpufreq
[   43.595271] CPU: 5 PID: 55 Comm: kworker/u16:2 Tainted: GW   
4.3.0-rc2+ #3
[   43.595272] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
[   43.595276] Workqueue: events_unbound async_run_entry_fn
[   43.595281]  8194ff67 88042a223b60 812c758a 

[   43.595286]  88042a223b98 810534c1 880429eca000 
880429de1000
[   43.595291]  880429de1000  0003 
88042a223ba8
[   43.595293] Call Trace:
[   43.595295]  [] dump_stack+0x4e/0x84
[   43.595298]  [] warn_slowpath_common+0x91/0xd0
[   43.595300]  [] warn_slowpath_null+0x1a/0x20
[   43.595303]  [] drm_helper_choose_crtc_dpms+0x91/0xa0
[   43.595315]  [] ? atombios_blank_crtc+0x140/0x140 [radeon]
[   43.595322]  [] drm_helper_connector_dpms+0xc4/0x110
[   43.595331]  [] radeon_suspend_kms+0x6b/0x380 [radeon]
[   43.595334]  [] ? _raw_spin_unlock_irqrestore+0x4b/0x80
[   43.595337]  [] ? pci_pm_poweroff+0x100/0x100
[   43.595346]  [] radeon_pmops_freeze+0x1c/0x20 [radeon]
[   43.595349]  [] pci_pm_freeze+0x6a/0x100
[   43.595351]  [] ? 

[PATCH] drm: Use userspace compatible type in fourcc_mod_code macro

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

__u64 should be used instead of u64.

Feature originally added in:

commit e3eb3250d84ef97b766312345774367b6a310db8
Author: Rob Clark 
Date:   Thu Feb 5 14:41:52 2015 +

drm: add support for tiled/compressed/etc modifier in addfb2

Signed-off-by: Tvrtko Ursulin 
Cc: Rob Clark 
Cc: Daniel Stone 
Cc: Daniel Vetter 
Cc: dri-devel at lists.freedesktop.org
Cc: stable at vger.kernel.org
---
 include/uapi/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 8c5e8b91a3cb..0b69a7753558 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -158,7 +158,7 @@
 /* add more to the end as needed */

 #define fourcc_mod_code(vendor, val) \
-   u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
0x00ffULL))
+   __u64)DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | (val & 
0x00ffULL))

 /*
  * Format Modifier tokens:
-- 
2.5.1

[PATCH 02/23] drm: Add structure for querying palette color capabilities

[Smith, Gary K]

Given that its only one word of info per LUT, I'm OK with it being two separate 
properties. 
I believe it was much more complex previously with a lot more info per LUT, 
which is probably why I preferred a blob.

Thanks
Gary

-Original Message-
From: Sharma, Shashank 
Sent: Wednesday, September 23, 2015 9:10 AM
To: Daniel Vetter; Roper, Matthew D
Cc: Matheson, Annie J; Bradford, Robert; Palleti, Avinash Reddy; intel-gfx at 
lists.freedesktop.org; dri-devel at lists.freedesktop.org; Mukherjee, Indranil; 
Bish, Jim; Smith, Gary K; kausalmalladi at gmail.com; Vetter, Daniel
Subject: Re: [PATCH 02/23] drm: Add structure for querying palette color 
capabilities

Hi Matt, Daniel
Addressing the review comments from both of you here.

Regards
Shashank

On 9/22/2015 6:32 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 10:51:31AM -0700, Matt Roper wrote:
>> On Wed, Sep 16, 2015 at 11:06:59PM +0530, Shashank Sharma wrote:
>>> From: Kausal Malladi 
>>>
>>> The DRM color management framework is targeting various hardware 
>>> platforms and drivers. Different platforms can have different color 
>>> correction and enhancement capabilities.
>>>
>>> A commom user space application can query these capabilities using 
>>> the DRM property interface. Each driver can fill this property with 
>>> its platform's palette color capabilities.
>>>
>>> This patch adds new structure in DRM layer for querying palette 
>>> color capabilities. This structure will be used by all user space 
>>> agents to configure appropriate color configurations.
>>>
>>> Signed-off-by: Shashank Sharma 
>>> Signed-off-by: Kausal Malladi 
>>
>> I think you provided an explanation on a previous code review cycle, 
>> but I forget the details now...what's the benefit to using a blob for 
>> caps rather than having these be individual properties?  Individual 
>> properties seems more natural to me, but I think you had a 
>> justification for blobbing them together; that reasoning would be 
>> good to include in the commit message.
>
> Yeah I'm leaning slightly towards individual props too, that would 
> give us a bit more freedom with placing them (e.g. if someone comes up 
> with funky hw where before_ctm and ctm are per-plane and after_ctm is 
> on the crtc, with only some planes support the before_ctm gamma table).
This was the part where we spent most of the time during the design review, and 
the reason we came up for this was:
- This is a read only property, which userspace would like to read only once, 
and cache the information. It was also Gary's opinion to keep this as single 
blob for all.
- Making individual property needs more information to be provided to user 
space.
- This is a blob only for pipe level capabilities, the plane level blob will be 
separate from this.
- We can handle this HW also, by loading proper plane and pipe level capability 
blob. This is more convenient to have all the capabilities together at the same 
place, than keep on querying the same.
>
> Also if you do per-prop properties instead of the blob you can drop 
> the version/reserved fields, since properties are inheritedly designed 
> to be extendible. So no need to revision them again (it only leads to 
> more code that might break).
> -Daniel
>
We are anyways planning to drop the version, as per Ville's comment.
- Shashank
>>
>>
>> Matt
>>
>>> ---
>>>   include/uapi/drm/drm.h | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 
>>> 3801584..e3c642f 100644
>>> --- a/include/uapi/drm/drm.h
>>> +++ b/include/uapi/drm/drm.h
>>> @@ -829,6 +829,17 @@ struct drm_event_vblank {
>>> __u32 reserved;
>>>   };
>>>
>>> +struct drm_palette_caps {
>>> +   /* Structure version. Should be 1 currently */
>>> +   __u32 version;
>>> +   /* For padding and future use */
>>> +   __u32 reserved;
>>> +   /* This may be 0 if not supported. e.g. plane palette or VLV pipe */
>>> +   __u32 num_samples_before_ctm;
>>> +   /* This will be non-zero for pipe. May be zero for planes on some HW */
>>> +   __u32 num_samples_after_ctm;
>>> +};
>>> +
>>>   /* typedef area */
>>>   #ifndef __KERNEL__
>>>   typedef struct drm_clip_rect drm_clip_rect_t;
>>> --
>>> 1.9.1
>>>
>>
>> --
>> Matt Roper
>> Graphics Software Engineer
>> IoTG Platform Enabling & Development
>> Intel Corporation
>> (916) 356-2795
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and 

WARNING: CPU: 4 PID: 863 at include/drm/drm_crtc.h:1577 drm_helper_choose_encoder_dpms+0x88/0x90()

[Daniel Vetter]

On Tue, Sep 22, 2015 at 04:54:54PM -0400, Alex Deucher wrote:
> On Tue, Sep 22, 2015 at 4:21 PM, Borislav Petkov  wrote:
> > Hi Alex,
> >
> > On Tue, Sep 22, 2015 at 03:58:03PM -0400, Alex Deucher wrote:
> >> What system is this?
> >
> > my workstation - an
> >
> > "To be filled by O.E.M. To be filled by O.E.M./M5A97 EVO R2.0, BIOS 1503 
> > 01/16/2013"
> >
> > you gotta love the "To be filled" crap. In any case, it is an ASUS M5A97
> > EVO R2.0. RD890 chip AFAICT.
> >
> >> What GPU are you using?
> >
> > RV635. Here's some dmesg:
> >
> > [6.489016] [drm] initializing kernel modesetting (RV635 0x1002:0x9598 
> > 0x1043:0x01DA).
> > [7.509177] radeon :01:00.0: VRAM: 512M 0x - 
> > 0x1FFF (512M used)
> > [7.518010] radeon :01:00.0: GTT: 512M 0x2000 - 
> > 0x3FFF
> > [7.525724] [drm] Detected VRAM RAM=512M, BAR=256M
> > [7.530608] [drm] RAM width 128bits DDR
> > [7.535168] [TTM] Zone  kernel: Available graphics memory: 8132226 kiB
> > [7.541779] [TTM] Zone   dma32: Available graphics memory: 2097152 kiB
> > [7.548420] [TTM] Initializing pool allocator
> > [7.552896] [TTM] Initializing DMA pool allocator
> > [7.558176] [drm] radeon: 512M of VRAM memory ready
> > [7.563131] [drm] radeon: 512M of GTT memory ready.
> > [7.568151] [drm] Loading RV635 Microcode
> > [7.577382] [drm] Internal thermal controller without fan control
> > [7.584349] [drm] radeon: power management initialized
> > [7.590443] [drm] GART: num cpu pages 131072, num gpu pages 131072
> > [7.597266] [drm] enabling PCIE gen 2 link speeds, disable with 
> > radeon.pcie_gen2=0
> > [7.624386] [drm] PCIE GART of 512M enabled (table at 
> > 0x00254000).
> > [7.631544] radeon :01:00.0: WB enabled
> > [7.635794] radeon :01:00.0: fence driver on ring 0 use gpu addr 
> > 0x2c00 and cpu addr 0x880427ef7c00
> > [7.647039] radeon :01:00.0: fence driver on ring 5 use gpu addr 
> > 0x000521d0 and cpu addr 0xc98121d0
> > [7.657924] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> > [7.664601] [drm] Driver supports precise vblank timestamp query.
> > [7.670780] radeon :01:00.0: radeon: MSI limited to 32-bit
> > [7.676801] radeon :01:00.0: radeon: using MSI.
> > [7.681863] [drm] radeon: irq initialized.
> > [7.717757] [drm] ring test on 0 succeeded in 0 usecs
> > [7.897466] [drm] ring test on 5 succeeded in 1 usecs
> > [7.902585] [drm] UVD initialized successfully.
> > [7.908108] [drm] ib test on ring 0 succeeded in 0 usecs
> > [8.558968] [drm] ib test on ring 5 succeeded
> > [8.568734] [drm] Radeon Display Connectors
> > [8.573005] [drm] Connector 0:
> > [8.576189] [drm]   DVI-I-1
> > [8.579062] [drm]   HPD1
> > [8.581657] [drm]   DDC: 0x7e50 0x7e50 0x7e54 0x7e54 0x7e58 0x7e58 
> > 0x7e5c 0x7e5c
> > [8.589172] [drm]   Encoders:
> > [8.592234] [drm] DFP1: INTERNAL_UNIPHY
> > [8.596492] [drm] CRT2: INTERNAL_KLDSCP_DAC2
> > [8.601182] [drm] Connector 1:
> > [8.604302] [drm]   DIN-1
> > [8.607012] [drm]   Encoders:
> > [8.610043] [drm] TV1: INTERNAL_KLDSCP_DAC2
> > [8.614642] [drm] Connector 2:
> > [8.617760] [drm]   DVI-I-2
> > [8.620621] [drm]   HPD2
> > [8.623226] [drm]   DDC: 0x7e40 0x7e40 0x7e44 0x7e44 0x7e48 0x7e48 
> > 0x7e4c 0x7e4c
> > [8.630719] [drm]   Encoders:
> > [8.633749] [drm] CRT1: INTERNAL_KLDSCP_DAC1
> > [8.638436] [drm] DFP2: INTERNAL_KLDSCP_LVTMA
> > [8.719815] [drm] fb mappable at 0xC0355000
> > [8.724089] [drm] vram apper at 0xC000
> > [8.728243] [drm] size 9216000
> > [8.731371] [drm] fb depth is 24
> > [8.734664] [drm]pitch is 7680
> > [8.739009] fbcon: radeondrmfb (fb0) is primary device
> > [8.802887] Console: switching to colour frame buffer device 240x75
> > [8.818487] radeon :01:00.0: fb0: radeondrmfb frame buffer device
> > [8.824948] radeon :01:00.0: registered panic notifier
> > [8.846452] [drm] Initialized radeon 2.42.0 20080528 for :01:00.0 on 
> > minor 0
> >
> >> Can you bisect?
> >
> > It is my workstation so it will take longer but I'll try.
> >
> > If you can think of some particular commits I should try, let me know.
> 
> Sorry, I can't think of anything off hand.  I suspect it was some
> change or cleanup in the core drm code.

The locking check is new, but I was only adding locking checks, not yet
reworking the locking itself. So the backtrace is likely (but not 100%
guaranteed) a red herring.

Strange thing is that I've tested this on a radeon over here and I don't
see this backtrace ... wut. Below diff should appease the backtraces at
least.
-Daniel

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index d8319dae8358..9f05de73ae97 100644
--- 

[regression] Re: [Linux-v4.2-10463-g9a9952bbd76a] i915: WARNING: intel_display.c:1377 assert_planes_disabled

[Daniel Vetter]

Adding Jairo to track this regression.
-Daniel

On Wed, Sep 23, 2015 at 08:23:04AM +0200, Sedat Dilek wrote:
> On Sun, Sep 13, 2015 at 9:06 AM, Sedat Dilek  wrote:
> > On Wed, Sep 9, 2015 at 4:42 AM, Sedat Dilek  
> > wrote:
> >> [ TO INTEL DRM DRIVERS maintainers ]
> >>
> >> Hi,
> >>
> >> out of curiosity and to play with the new bindeb-pkg make-target I
> >> built pre-v4.3-rc1 (git-describe says v4.2-10463-g9a9952bbd76a)
> >> Debian-kernel packages.
> >>
> >> I see several bugs and call-traces.
> >>
> >> Two hit a warning in i915 (one snippet here, full dmesg-log and
> >> kernel-config are attached).
> >>
> >> [   20.920943] [ cut here ]
> >> [   20.920982] WARNING: CPU: 0 PID: 114 at
> >> drivers/gpu/drm/i915/intel_display.c:1377
> >> assert_planes_disabled+0xe4/0x150 [i915]()
> >> [   20.920983] plane A assertion failure, should be disabled but not
> >> [   20.921023] Modules linked in: i915 mac80211 snd_hda_codec_hdmi
> >> snd_hda_codec_realtek snd_hda_codec_generic uvcvideo snd_hda_intel
> >> snd_hda_codec videobuf2_vmalloc videobuf2_memops videobuf2_core rfcomm
> >> joydev bnep v4l2_common snd_hwdep iwlwifi videodev usb_storage
> >> kvm_intel snd_hda_core parport_pc cfg80211 btusb i2c_algo_bit
> >> drm_kms_helper ppdev btrtl btbcm snd_pcm btintel kvm bluetooth
> >> snd_seq_midi psmouse snd_seq_midi_event snd_rawmidi syscopyarea
> >> snd_seq sysfillrect sysimgblt fb_sys_fops drm snd_timer snd_seq_device
> >> serio_raw samsung_laptop snd lpc_ich video soundcore wmi intel_rst
> >> mac_hid lp parport binfmt_misc hid_generic usbhid hid r8169 mii
> >> [   20.921026] CPU: 0 PID: 114 Comm: kworker/u16:5 Not tainted
> >> 4.2.0-10463.1-iniza-small #1
> >> [   20.921027] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> >> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
> >> [   20.921033] Workqueue: events_unbound async_run_entry_fn
> >> [   20.921037]  a06a9d60 8800378af718 813df89b
> >> 8800378af760
> >> [   20.921039]  8800378af750 8107cdc6 8800c200
> >> 
> >> [   20.921042]   8800bf90b000 8800bf90b000
> >> 8800378af7b0
> >> [   20.921043] Call Trace:
> >> [   20.921046]  [] dump_stack+0x4b/0x70
> >> [   20.921050]  [] warn_slowpath_common+0x86/0xc0
> >> [   20.921052]  [] warn_slowpath_fmt+0x4c/0x50
> >> [   20.921081]  [] assert_planes_disabled+0xe4/0x150 
> >> [i915]
> >> [   20.921106]  [] intel_disable_pipe+0x4f/0x2d0 [i915]
> >> [   20.921129]  [] ironlake_crtc_disable+0x85/0x7d0 
> >> [i915]
> >> [   20.921151]  [] ?
> >> intel_crtc_disable_planes+0xde/0xf0 [i915]
> >> [   20.921176]  [] intel_atomic_commit+0x108/0x1370 
> >> [i915]
> >> [   20.921192]  [] ? drm_atomic_check_only+0x1d7/0x5a0 
> >> [drm]
> >> [   20.921205]  [] ?
> >> drm_atomic_get_connector_state+0x49/0x110 [drm]
> >> [   20.921216]  [] drm_atomic_commit+0x37/0x60 [drm]
> >> [   20.921223]  []
> >> drm_atomic_helper_set_config+0x1ca/0x430 [drm_kms_helper]
> >> [   20.921234]  []
> >> drm_mode_set_config_internal+0x65/0x110 [drm]
> >> [   20.921240]  [] restore_fbdev_mode+0xbe/0xe0
> >> [drm_kms_helper]
> >> [   20.921246]  []
> >> drm_fb_helper_restore_fbdev_mode_unlocked+0x25/0x70 [drm_kms_helper]
> >> [   20.921251]  [] drm_fb_helper_set_par+0x2d/0x50
> >> [drm_kms_helper]
> >> [   20.921280]  [] intel_fbdev_set_par+0x1a/0x60 [i915]
> >> [   20.921283]  [] fbcon_init+0x53e/0x5d0
> >> [   20.921286]  [] visual_init+0xca/0x130
> >> [   20.921289]  [] do_bind_con_driver+0x167/0x3a0
> >> [   20.921292]  [] do_take_over_console+0xac/0x1d0
> >> [   20.921294]  [] do_fbcon_takeover+0x57/0xb0
> >> [   20.921296]  [] fbcon_event_notify+0x752/0x860
> >> [   20.921299]  [] ? 
> >> __blocking_notifier_call_chain+0x35/0x70
> >> [   20.921301]  [] notifier_call_chain+0x5d/0x80
> >> [   20.921304]  [] 
> >> __blocking_notifier_call_chain+0x4d/0x70
> >> [   20.921306]  [] blocking_notifier_call_chain+0x16/0x20
> >> [   20.921308]  [] fb_notifier_call_chain+0x1b/0x20
> >> [   20.921311]  [] register_framebuffer+0x204/0x330
> >> [   20.921317]  []
> >> drm_fb_helper_initial_config+0x234/0x3b0 [drm_kms_helper]
> >> [   20.921344]  [] intel_fbdev_initial_config+0x1b/0x20 
> >> [i915]
> >> [   20.921346]  [] async_run_entry_fn+0x37/0xe0
> >> [   20.921349]  [] process_one_work+0x1e1/0x620
> >> [   20.921352]  [] ? process_one_work+0x156/0x620
> >> [   20.921353]  [] worker_thread+0x69/0x480
> >> [   20.921355]  [] ? cancel_delayed_work_sync+0x20/0x20
> >> [   20.921357]  [] kthread+0x10a/0x120
> >> [   20.921360]  [] ? kthread_create_on_node+0x200/0x200
> >> [   20.921364]  [] ret_from_fork+0x3f/0x70
> >> [   20.921366]  [] ? kthread_create_on_node+0x200/0x200
> >> [   20.921367] ---[ end trace 85fce5c152ff4528 ]---
> >>
> >> Some intelgfx hardware infos (see attached Xorg.0.log)...
> >>
> >> [27.487] (--) intel(0): Integrated Graphics Chipset: Intel(R) HD
> >> Graphics 3000
> >> [27.565] (II) intel(0): SNA 

[PATCH v1.1 03/15] vga_switcheroo: Set active attribute to false for audio clients

[Daniel Vetter]

On Thu, Aug 27, 2015 at 04:43:43PM +0200, Lukas Wunner wrote:
> The active attribute in struct vga_switcheroo_client denotes whether
> the outputs are currently switched to this client. The attribute is
> only meaningful for vga clients. It is never used for audio clients.
> 
> The function vga_switcheroo_register_audio_client() misuses this
> attribute to store whether the audio device is fully initialized.
> Most likely there was a misunderstanding about the meaning of
> "active" when this was added.
> 
> Set the active attribute to false for audio clients. Remove the
> active parameter from vga_switcheroo_register_audio_client() and
> its sole caller, hda_intel.c:register_vga_switcheroo().
> 
> vga_switcheroo_register_audio_client() was introduced by 3e9e63dbd374
> ("vga_switcheroo: Add the support for audio clients"). Its use in
> hda_intel.c was introduced by a82d51ed24bb ("ALSA: hda - Support
> VGA-switcheroo").
> 
> v1.1: The changes above imply that in find_active_client() the call
> to client_is_vga() is now superfluous. Drop it.
> 
> Cc: Takashi Iwai 
> Signed-off-by: Lukas Wunner 

Takashi, can you pls ack this for merging through drm-misc? Patch lgtm.
-Daniel

> ---
>  drivers/gpu/vga/vga_switcheroo.c | 7 +++
>  include/linux/vga_switcheroo.h   | 4 ++--
>  sound/pci/hda/hda_intel.c| 3 +--
>  3 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/vga/vga_switcheroo.c 
> b/drivers/gpu/vga/vga_switcheroo.c
> index 2e7e2f8e..563b82f 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -290,7 +290,6 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
>   * @pdev: client pci device
>   * @ops: client callbacks
>   * @id: client identifier, see enum vga_switcheroo_client_id
> - * @active: whether the audio device is fully initialized
>   *
>   * Register audio client (audio device on a GPU). The power state of the
>   * client is assumed to be ON.
> @@ -299,9 +298,9 @@ EXPORT_SYMBOL(vga_switcheroo_register_client);
>   */
>  int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
>const struct vga_switcheroo_client_ops 
> *ops,
> -  int id, bool active)
> +  int id)
>  {
> - return register_client(pdev, ops, id | ID_BIT_AUDIO, active, false);
> + return register_client(pdev, ops, id | ID_BIT_AUDIO, false, false);
>  }
>  EXPORT_SYMBOL(vga_switcheroo_register_audio_client);
>  
> @@ -333,7 +332,7 @@ find_active_client(struct list_head *head)
>   struct vga_switcheroo_client *client;
>  
>   list_for_each_entry(client, head, list)
> - if (client->active && client_is_vga(client))
> + if (client->active)
>   return client;
>   return NULL;
>  }
> diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
> index fe90bfc..3764991 100644
> --- a/include/linux/vga_switcheroo.h
> +++ b/include/linux/vga_switcheroo.h
> @@ -128,7 +128,7 @@ int vga_switcheroo_register_client(struct pci_dev *dev,
>  bool driver_power_control);
>  int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
>const struct vga_switcheroo_client_ops 
> *ops,
> -  int id, bool active);
> +  int id);
>  
>  void vga_switcheroo_client_fb_set(struct pci_dev *dev,
> struct fb_info *info);
> @@ -154,7 +154,7 @@ static inline void vga_switcheroo_client_fb_set(struct 
> pci_dev *dev, struct fb_i
>  static inline int vga_switcheroo_register_handler(struct 
> vga_switcheroo_handler *handler) { return 0; }
>  static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
>   const struct vga_switcheroo_client_ops *ops,
> - int id, bool active) { return 0; }
> + int id) { return 0; }
>  static inline void vga_switcheroo_unregister_handler(void) {}
>  static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
>  static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { 
> return VGA_SWITCHEROO_ON; }
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index c38c68f..e819013 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1143,8 +1143,7 @@ static int register_vga_switcheroo(struct azx *chip)
>* is there any machine with two switchable HDMI audio controllers?
>*/
>   err = vga_switcheroo_register_audio_client(chip->pci, _vs_ops,
> - VGA_SWITCHEROO_DIS,
> - hda->probe_continued);
> +VGA_SWITCHEROO_DIS);
>   if (err < 0)
>   return err;
>   hda->vga_switcheroo_registered = 1;
> -- 
> 1.8.5.2 (Apple Git-48)
> 

-- 

[Bug 92087] [Radeon/Tonga] lockup in valley demo

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

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

Bug ID: 92087
   Summary: [Radeon/Tonga] lockup in valley demo
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: edward.ocallaghan at koparo.com

Hi,

Upon running the valley demo I experience I lockup after a while. I can recover
by switching to another virtual term and killing X.

dmesg shows this repeatedly with only a difference of the value after the fault
id? of 146 after the lockup:



VM fault (0x00, vmid 0) at page 0, read from '' (0x) (0)
amdgpu :01:00.0: GPU fault detected: 146 0x02704004
amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR   0x
amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x



This is with mesa 11.0.1 (8ae8feca) + kern 4.3.0-rc2 + libdrm 2.4.65

No idea how to get a card state dump with amdgpu?

Edward.

-- 
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/20150923/f3ec4b71/attachment.html>

[PATCH 01/23] drm: Create Color Management DRM properties

[Sharma, Shashank]

Sure Matt, 
We are planning to add documentation for color properties, as suggested by you 
and Daniel. 

Regards
Shashank
-Original Message-
From: Roper, Matthew D 
Sent: Wednesday, September 16, 2015 11:21 PM
To: Sharma, Shashank
Cc: Bish, Jim; Bradford, Robert; Smith, Gary K; dri-devel at 
lists.freedesktop.org; intel-gfx at lists.freedesktop.org; Vetter, Daniel; 
Matheson, Annie J; Mukherjee, Indranil; Palleti, Avinash Reddy; kausalmalladi 
at gmail.com
Subject: Re: [PATCH 01/23] drm: Create Color Management DRM properties

On Wed, Sep 16, 2015 at 11:06:58PM +0530, Shashank Sharma wrote:
> From: Kausal Malladi 
> 
> Color Management is an extension to Kernel display framework. It 
> allows abstraction of hardware color correction and enhancement 
> capabilities by virtue of DRM properties.
> 
> This patch initializes color management framework by :
> 1. Introducing new pointers in DRM mode_config structure to
>carry CTM and Palette color correction properties.
> 2. Creating these DRM properties in DRM standard properties creation
>sequence.
> 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Kausal Malladi 

We should probably update the property section of 
Documentation/DocBook/drm.tmpl with this patch as well to include these new 
properties in the table (that docbook ultimately generates documentation that 
looks like https://kernel.org/doc/htmldocs/drm/drm-kms-properties.html ).

One minor note:  people not involved in color management probably won't 
immediately figure out what "CTM" stands for, so you might want to just add a 
comment somewhere that spells out the full "color transformation matrix" term.


Matt


> ---
>  drivers/gpu/drm/drm_crtc.c | 26 ++
>  include/drm/drm_crtc.h |  6 ++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c 
> index 9b9c4b4..d809c67 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1472,6 +1472,32 @@ static int drm_mode_create_standard_properties(struct 
> drm_device *dev)
>   return -ENOMEM;
>   dev->mode_config.prop_mode_id = prop;
>  
> + /* Color Management properties */
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB | DRM_MODE_PROP_IMMUTABLE,
> + "CRTC_PALETTE_CAPABILITIES", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_crtc_palette_capabilities_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "PALETTE_AFTER_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_palette_after_ctm_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "PALETTE_BEFORE_CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_palette_before_ctm_property = prop;
> +
> + prop = drm_property_create(dev,
> + DRM_MODE_PROP_BLOB, "CTM", 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.cm_ctm_property = prop;
> +
>   return 0;
>  }
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 
> c0366e9..c35531e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1153,6 +1153,12 @@ struct drm_mode_config {
>   struct drm_property *suggested_x_property;
>   struct drm_property *suggested_y_property;
>  
> + /* Color Management Properties */
> + struct drm_property *cm_crtc_palette_capabilities_property;
> + struct drm_property *cm_palette_before_ctm_property;
> + struct drm_property *cm_palette_after_ctm_property;
> + struct drm_property *cm_ctm_property;
> +
>   /* dumb ioctl parameters */
>   uint32_t preferred_depth, prefer_shadow;
>  
> --
> 1.9.1
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[Linux-v4.2-10463-g9a9952bbd76a] i915: WARNING: intel_display.c:1377 assert_planes_disabled

[Sedat Dilek]

On Sun, Sep 13, 2015 at 9:06 AM, Sedat Dilek  wrote:
> On Wed, Sep 9, 2015 at 4:42 AM, Sedat Dilek  wrote:
>> [ TO INTEL DRM DRIVERS maintainers ]
>>
>> Hi,
>>
>> out of curiosity and to play with the new bindeb-pkg make-target I
>> built pre-v4.3-rc1 (git-describe says v4.2-10463-g9a9952bbd76a)
>> Debian-kernel packages.
>>
>> I see several bugs and call-traces.
>>
>> Two hit a warning in i915 (one snippet here, full dmesg-log and
>> kernel-config are attached).
>>
>> [   20.920943] [ cut here ]
>> [   20.920982] WARNING: CPU: 0 PID: 114 at
>> drivers/gpu/drm/i915/intel_display.c:1377
>> assert_planes_disabled+0xe4/0x150 [i915]()
>> [   20.920983] plane A assertion failure, should be disabled but not
>> [   20.921023] Modules linked in: i915 mac80211 snd_hda_codec_hdmi
>> snd_hda_codec_realtek snd_hda_codec_generic uvcvideo snd_hda_intel
>> snd_hda_codec videobuf2_vmalloc videobuf2_memops videobuf2_core rfcomm
>> joydev bnep v4l2_common snd_hwdep iwlwifi videodev usb_storage
>> kvm_intel snd_hda_core parport_pc cfg80211 btusb i2c_algo_bit
>> drm_kms_helper ppdev btrtl btbcm snd_pcm btintel kvm bluetooth
>> snd_seq_midi psmouse snd_seq_midi_event snd_rawmidi syscopyarea
>> snd_seq sysfillrect sysimgblt fb_sys_fops drm snd_timer snd_seq_device
>> serio_raw samsung_laptop snd lpc_ich video soundcore wmi intel_rst
>> mac_hid lp parport binfmt_misc hid_generic usbhid hid r8169 mii
>> [   20.921026] CPU: 0 PID: 114 Comm: kworker/u16:5 Not tainted
>> 4.2.0-10463.1-iniza-small #1
>> [   20.921027] Hardware name: SAMSUNG ELECTRONICS CO., LTD.
>> 530U3BI/530U4BI/530U4BH/530U3BI/530U4BI/530U4BH, BIOS 13XK 03/28/2013
>> [   20.921033] Workqueue: events_unbound async_run_entry_fn
>> [   20.921037]  a06a9d60 8800378af718 813df89b
>> 8800378af760
>> [   20.921039]  8800378af750 8107cdc6 8800c200
>> 
>> [   20.921042]   8800bf90b000 8800bf90b000
>> 8800378af7b0
>> [   20.921043] Call Trace:
>> [   20.921046]  [] dump_stack+0x4b/0x70
>> [   20.921050]  [] warn_slowpath_common+0x86/0xc0
>> [   20.921052]  [] warn_slowpath_fmt+0x4c/0x50
>> [   20.921081]  [] assert_planes_disabled+0xe4/0x150 [i915]
>> [   20.921106]  [] intel_disable_pipe+0x4f/0x2d0 [i915]
>> [   20.921129]  [] ironlake_crtc_disable+0x85/0x7d0 [i915]
>> [   20.921151]  [] ?
>> intel_crtc_disable_planes+0xde/0xf0 [i915]
>> [   20.921176]  [] intel_atomic_commit+0x108/0x1370 [i915]
>> [   20.921192]  [] ? drm_atomic_check_only+0x1d7/0x5a0 
>> [drm]
>> [   20.921205]  [] ?
>> drm_atomic_get_connector_state+0x49/0x110 [drm]
>> [   20.921216]  [] drm_atomic_commit+0x37/0x60 [drm]
>> [   20.921223]  []
>> drm_atomic_helper_set_config+0x1ca/0x430 [drm_kms_helper]
>> [   20.921234]  []
>> drm_mode_set_config_internal+0x65/0x110 [drm]
>> [   20.921240]  [] restore_fbdev_mode+0xbe/0xe0
>> [drm_kms_helper]
>> [   20.921246]  []
>> drm_fb_helper_restore_fbdev_mode_unlocked+0x25/0x70 [drm_kms_helper]
>> [   20.921251]  [] drm_fb_helper_set_par+0x2d/0x50
>> [drm_kms_helper]
>> [   20.921280]  [] intel_fbdev_set_par+0x1a/0x60 [i915]
>> [   20.921283]  [] fbcon_init+0x53e/0x5d0
>> [   20.921286]  [] visual_init+0xca/0x130
>> [   20.921289]  [] do_bind_con_driver+0x167/0x3a0
>> [   20.921292]  [] do_take_over_console+0xac/0x1d0
>> [   20.921294]  [] do_fbcon_takeover+0x57/0xb0
>> [   20.921296]  [] fbcon_event_notify+0x752/0x860
>> [   20.921299]  [] ? 
>> __blocking_notifier_call_chain+0x35/0x70
>> [   20.921301]  [] notifier_call_chain+0x5d/0x80
>> [   20.921304]  [] __blocking_notifier_call_chain+0x4d/0x70
>> [   20.921306]  [] blocking_notifier_call_chain+0x16/0x20
>> [   20.921308]  [] fb_notifier_call_chain+0x1b/0x20
>> [   20.921311]  [] register_framebuffer+0x204/0x330
>> [   20.921317]  []
>> drm_fb_helper_initial_config+0x234/0x3b0 [drm_kms_helper]
>> [   20.921344]  [] intel_fbdev_initial_config+0x1b/0x20 
>> [i915]
>> [   20.921346]  [] async_run_entry_fn+0x37/0xe0
>> [   20.921349]  [] process_one_work+0x1e1/0x620
>> [   20.921352]  [] ? process_one_work+0x156/0x620
>> [   20.921353]  [] worker_thread+0x69/0x480
>> [   20.921355]  [] ? cancel_delayed_work_sync+0x20/0x20
>> [   20.921357]  [] kthread+0x10a/0x120
>> [   20.921360]  [] ? kthread_create_on_node+0x200/0x200
>> [   20.921364]  [] ret_from_fork+0x3f/0x70
>> [   20.921366]  [] ? kthread_create_on_node+0x200/0x200
>> [   20.921367] ---[ end trace 85fce5c152ff4528 ]---
>>
>> Some intelgfx hardware infos (see attached Xorg.0.log)...
>>
>> [27.487] (--) intel(0): Integrated Graphics Chipset: Intel(R) HD
>> Graphics 3000
>> [27.565] (II) intel(0): SNA initialized with Sandybridge (gen6, gt2) 
>> backend
>>
>> Please have a look at it.
>>
>> If this is a known issue, please let me know and kindly point me to a
>> patch (patchwork preferred) or commit-id in drm-intel Git.
>> ( Sorry, I have not checked any mailing-lists or patchwork. )
>>
>> If you need more infos, logs and/or debug 

[Bug 92086] AMD Trinity No screen at HDMI after S3 wakeup

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

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

Bug ID: 92086
   Summary: AMD Trinity No screen at HDMI after S3 wakeup
   Product: DRI
   Version: XOrg git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: DRM/Radeon
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: vinibali at freemail.hu

Created attachment 118407
  --> https://bugs.freedesktop.org/attachment.cgi?id=118407=edit
trinity_wakeup.log

Dear Devs,
ive got this really annoying issue with multiple boards:
- ASRock FM2A75 Pro4
- ASRock A88M-HD+
- ASUS A78M-E
The simptome is the same in every board, but just with A8-7800K Trinity(ARUBA).
I've also tested with A10-7700K Kaveri(GCN1.1), but there was no problems at
all. So I put the PC to S3 sleep, than wake it up. The monitor which uses DVI
connector, works perfectly, but the HDMI one is mostly black,stays at sleep,
doesnt displays information(out of range, etc), but sometimes a really
flickering screen comes in for a few momements than disappears.
I hope its still a non-listed bug, I've checked multiple reports, but doesnt
find thisone. Look at the attachments, dmesg log added.
Thanks

-- 
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/20150923/84dca1de/attachment-0001.html>

[patch 4/4 v2] drm/qxl: integer overflow in qxl_alloc_surf_ioctl()

[Frediano Ziglio]

> 
> The size calculation can overflow.  I don't know if this leads to
> memory corruption, but it causes a static checker warning.
> 
> Signed-off-by: Dan Carpenter 
> ---
> v2: I don't know think the size is capped anywhere.  In my first version
> of this patch, I introduced a divide by zero bug.
> 

Beside the second sentence I would ack

Frediano

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c
> b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index b2db482..49b3158 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
> @@ -396,12 +396,14 @@ static int qxl_alloc_surf_ioctl(struct drm_device *dev,
> void *data,
>   struct qxl_bo *qobj;
>   int handle;
>   int ret;
> - int size, actual_stride;
> + u64 size, actual_stride;
>   struct qxl_surface surf;
>  
>   /* work out size allocate bo with handle */
>   actual_stride = param->stride < 0 ? -param->stride : param->stride;
>   size = actual_stride * param->height + actual_stride;
> + if (size > INT_MAX)
> + return -EINVAL;
>  
>   surf.format = param->format;
>   surf.width = param->width;
> 

[Bug 104881] AMDGPU FIJI doesn't support higher resolutions past 1920x1080

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

https://bugzilla.kernel.org/show_bug.cgi?id=104881

Jeff Nelson  changed:

   What|Removed |Added

   Hardware|Other   |x86-64

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 104881] AMDGPU FIJI doesn't support higher resolutions past 1920x1080

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

https://bugzilla.kernel.org/show_bug.cgi?id=104881

--- Comment #2 from Jeff Nelson  ---
Created attachment 188131
  --> https://bugzilla.kernel.org/attachment.cgi?id=188131=edit
dmesg

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 104881] AMDGPU FIJI doesn't support higher resolutions past 1920x1080

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

https://bugzilla.kernel.org/show_bug.cgi?id=104881

--- Comment #1 from Jeff Nelson  ---
Created attachment 188121
  --> https://bugzilla.kernel.org/attachment.cgi?id=188121=edit
Xorg.0.log

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 104881] New: AMDGPU FIJI doesn't support higher resolutions past 1920x1080

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

https://bugzilla.kernel.org/show_bug.cgi?id=104881

Bug ID: 104881
   Summary: AMDGPU FIJI doesn't support higher resolutions past
1920x1080
   Product: Drivers
   Version: 2.5
Kernel Version: 4.3.0-rc2-1.g2b75354-desktop
  Hardware: Other
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: longshot902 at gmail.com
Regression: No

Created attachment 188111
  --> https://bugzilla.kernel.org/attachment.cgi?id=188111=edit
xrandr

I currently own two Dell P2715Q monitors, each having a native resolution of
3840x2160. When using DisplayPort on my AMD RADEON R9 FURY X, the resolution is
not a known option. I previously had an AMD RADON R9 290X and it was able to
detect the correct resolution of both monitors.

Attached is my xrandr --verbose" to show that the AMDGPU is not handling the
correct resolution available.

Please also note that while 1920x1080 is a selectable option, it is running in
interlaced mode, 1080i.

I have had some success with creating my own monitor xrandr profile for each
screen. It does temporarily resolve the issue, but it not persistent after a
restart or at login.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH 0/4] some optimization for evergreen cs

[Grazvydas Ignotas]

On Sun, Aug 23, 2015 at 3:57 AM, Grazvydas Ignotas  wrote:
> These patches try to reduce CPU usage of register command checker
> without affecting functionality.
> For me this gives 3-4% perf improvement in glxgears and ~1% CPU usage 
> reduction
> in "The Talos Principle" CS thread.
>
> Grazvydas Ignotas (4):
>   drm/radeon: simplify register checker
>   drm/radeon: split evergreen_cs_check_reg
>   drm/radeon: refactor register check loop
>   drm/radeon: remove use of volatile qualifier
>
>  drivers/gpu/drm/radeon/evergreen_cs.c | 104 
> +++---
>  1 file changed, 47 insertions(+), 57 deletions(-)

Can someone take a look at these? They still apply on current mainline
and I've been using them for a while without issues. I've also ran
piglit gpu tests and found no regressions.

Gražvydas

[PATCH v2 3/3] drm: tegra: use of_get_i2c_adapter_by_node interface

[Vladimir Zapolskiy]

This change is needed to properly lock I2C bus driver, which serves DDC.

On release of_get_i2c_adapter_by_node() requires i2c_put_adapter() call.

Note, that prior to the change put_device() coupled with
of_find_i2c_adapter_by_node() was missing on error path of
tegra_output_probe().

Signed-off-by: Vladimir Zapolskiy 
Cc: Thierry Reding 
Cc: Terje Bergström 
---
Changes from v1 to v2:
- converted two of_node_put(ddc) calls into one

 drivers/gpu/drm/tegra/output.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 46664b6..9f3cec5 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -120,14 +120,12 @@ int tegra_output_probe(struct tegra_output *output)

ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
if (ddc) {
-   output->ddc = of_find_i2c_adapter_by_node(ddc);
+   output->ddc = of_get_i2c_adapter_by_node(ddc);
+   of_node_put(ddc);
if (!output->ddc) {
err = -EPROBE_DEFER;
-   of_node_put(ddc);
return err;
}
-
-   of_node_put(ddc);
}

output->hpd_gpio = of_get_named_gpio_flags(output->of_node,
@@ -140,14 +138,13 @@ int tegra_output_probe(struct tegra_output *output)
   "HDMI hotplug detect");
if (err < 0) {
dev_err(output->dev, "gpio_request_one(): %d\n", err);
-   return err;
+   goto i2c_release;
}

err = gpio_to_irq(output->hpd_gpio);
if (err < 0) {
dev_err(output->dev, "gpio_to_irq(): %d\n", err);
-   gpio_free(output->hpd_gpio);
-   return err;
+   goto gpio_release;
}

output->hpd_irq = err;
@@ -160,8 +157,7 @@ int tegra_output_probe(struct tegra_output *output)
if (err < 0) {
dev_err(output->dev, "failed to request IRQ#%u: %d\n",
output->hpd_irq, err);
-   gpio_free(output->hpd_gpio);
-   return err;
+   goto gpio_release;
}

output->connector.polled = DRM_CONNECTOR_POLL_HPD;
@@ -175,6 +171,12 @@ int tegra_output_probe(struct tegra_output *output)
}

return 0;
+
+ gpio_release:
+   gpio_free(output->hpd_gpio);
+ i2c_release:
+   i2c_put_adapter(output->ddc);
+   return err;
 }

 void tegra_output_remove(struct tegra_output *output)
@@ -184,8 +186,7 @@ void tegra_output_remove(struct tegra_output *output)
gpio_free(output->hpd_gpio);
}

-   if (output->ddc)
-   put_device(>ddc->dev);
+   i2c_put_adapter(output->ddc);
 }

 int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
-- 
2.5.0

[PATCH v2 2/3] drm: tilcdc: use of_get_i2c_adapter_by_node interface

[Vladimir Zapolskiy]

This change is needed to properly lock I2C bus driver, which serves DDC.

Prior to this change i2c_put_adapter() is misused, which may lead
to an overflow over zero of I2C bus driver user counter.

Signed-off-by: Vladimir Zapolskiy 
---
Changes from v1 to v2:
- none

 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c 
b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index 354c47c..4dc78c7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -348,15 +348,13 @@ static int tfp410_probe(struct platform_device *pdev)
goto fail;
}

-   tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
+   tfp410_mod->i2c = of_get_i2c_adapter_by_node(i2c_node);
+   of_node_put(i2c_node);
if (!tfp410_mod->i2c) {
dev_err(>dev, "could not get i2c\n");
-   of_node_put(i2c_node);
goto fail;
}

-   of_node_put(i2c_node);
-
tfp410_mod->gpio = of_get_named_gpio_flags(node, "powerdn-gpio",
0, NULL);
if (IS_ERR_VALUE(tfp410_mod->gpio)) {
-- 
2.5.0

[PATCH v2 1/3] drm: dw_hdmi: use of_get_i2c_adapter_by_node interface

[Vladimir Zapolskiy]

This change is needed to properly lock I2C bus driver, which serves DDC.

The change fixes an overflow over zero of I2C bus driver user counter:

root at mx6q:~# lsmod | grep i2c
i2c_imx15348  0
root at mx6q:~# lsmod | grep dw_hdmi_imx
dw_hdmi_imx 3567  0
dw_hdmi15850  1 dw_hdmi_imx
imxdrm  8610  3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb
root at mx6q:~# rmmod dw_hdmi_imx
root at mx6q:~# lsmod | grep i2c
i2c_imx15348  -1

 ^

root at mx6q:~# rmmod i2c_imx
rmmod: ERROR: Module i2c_imx is in use

Note that prior to this change put_device() coupled with
of_find_i2c_adapter_by_node() was missing on error path of
dw_hdmi_bind(), added i2c_put_adapter() there along with the change.

Signed-off-by: Vladimir Zapolskiy 
Cc: Russell King 
Cc: Philipp Zabel 
Cc: Andy Yan 
---
Changes from v1 to v2:
- none

 drivers/gpu/drm/bridge/dw_hdmi.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 0083d4e..c2d804f 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1638,7 +1638,7 @@ int dw_hdmi_bind(struct device *dev, struct device 
*master,

ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
if (ddc_node) {
-   hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
+   hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
of_node_put(ddc_node);
if (!hdmi->ddc) {
dev_dbg(hdmi->dev, "failed to read ddc node\n");
@@ -1650,20 +1650,22 @@ int dw_hdmi_bind(struct device *dev, struct device 
*master,
}

hdmi->regs = devm_ioremap_resource(dev, iores);
-   if (IS_ERR(hdmi->regs))
-   return PTR_ERR(hdmi->regs);
+   if (IS_ERR(hdmi->regs)) {
+   ret = PTR_ERR(hdmi->regs);
+   goto err_res;
+   }

hdmi->isfr_clk = devm_clk_get(hdmi->dev, "isfr");
if (IS_ERR(hdmi->isfr_clk)) {
ret = PTR_ERR(hdmi->isfr_clk);
dev_err(hdmi->dev, "Unable to get HDMI isfr clk: %d\n", ret);
-   return ret;
+   goto err_res;
}

ret = clk_prepare_enable(hdmi->isfr_clk);
if (ret) {
dev_err(hdmi->dev, "Cannot enable HDMI isfr clock: %d\n", ret);
-   return ret;
+   goto err_res;
}

hdmi->iahb_clk = devm_clk_get(hdmi->dev, "iahb");
@@ -1729,6 +1731,8 @@ err_iahb:
clk_disable_unprepare(hdmi->iahb_clk);
 err_isfr:
clk_disable_unprepare(hdmi->isfr_clk);
+err_res:
+   i2c_put_adapter(hdmi->ddc);

return ret;
 }
-- 
2.5.0

[PATCH v2 0/3] drm: fix i2c adapter device driver user counter

[Vladimir Zapolskiy]

of_find_i2c_adapter_by_node() call requires quite often missing
put_device(), and i2c_put_adapter() releases a device locked by
i2c_get_adapter() only.

Below is a common error reproduction scenario as a result of the
misusage described above (this is run on iMX6 platform with
HDMI and I2C bus drivers compiled as kernel modules for clearness):

root at mx6q:~# lsmod | grep i2c
i2c_imx15348  0
root at mx6q:~# lsmod | grep dw_hdmi_imx
dw_hdmi_imx 3567  0
dw_hdmi15850  1 dw_hdmi_imx
imxdrm  8610  3 dw_hdmi_imx,imx_ipuv3_crtc,imx_ldb
root at mx6q:~# rmmod dw_hdmi_imx
root at mx6q:~# lsmod | grep i2c
i2c_imx15348  -1

 ^

root at mx6q:~# rmmod i2c_imx
rmmod: ERROR: Module i2c_imx is in use

To fix existing users of these interfaces use of_get_i2c_adapter_by_node()
interface, which is similar to i2c_get_adapter() in sense that an I2C bus
device driver found and locked by a user can be correctly unlocked by
i2c_put_adapter() call.

Changes from v1 to v2:
- none, this series is a straightforward bugfix, v1 was a blend of
  I2C core changes, bugfixes and improvements

The change is based on dri/drm-next.

Vladimir Zapolskiy (3):
  drm: dw_hdmi: use of_get_i2c_adapter_by_node interface
  drm: tilcdc: use of_get_i2c_adapter_by_node interface
  drm: tegra: use of_get_i2c_adapter_by_node interface

 drivers/gpu/drm/bridge/dw_hdmi.c   | 14 +-
 drivers/gpu/drm/tegra/output.c | 23 ---
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |  6 ++
 3 files changed, 23 insertions(+), 20 deletions(-)

-- 
2.5.0