Re: [PATCH v3 0/8] Support DEVICE_GENERIC memory in migrate_vma_*
Am 2021-06-24 um 1:30 a.m. schrieb Christoph Hellwig: > On Wed, Jun 23, 2021 at 05:49:55PM -0400, Felix Kuehling wrote: >> For the reference counting changes we could use the dax driver with hmem >> and use efi_fake_mem on the kernel command line to create some >> DEVICE_GENERIC pages. I'm open to suggestions for good user mode tests to >> exercise dax functionality on this type of memory. >> >> For the migration helper changes we could modify or parametrize >> lib/hmm_test.c to create DEVICE_GENERIC pages instead of DEVICE_PRIVATE. >> Then run tools/testing/selftests/vm/hmm-tests.c. > We'll also need a real in-tree user of the enhanced DEVICE_GENERIC memory. > So while the refcounting cleanups early in the series are something I'd > really like to see upstream as soon as everything is sorted out, the > actual bits that can't only be used by your updated driver should wait > for that. The driver changes are pretty much ready to go. But we have a bit of a chicken-egg problem because those changes likely go through different trees. The GPU driver changes will go through drm-next, but we can't merge them there until our dependencies have been merged there from upstream. Unless we protect everything with some #ifdef. Regards, Felix ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 24.06.21 um 10:12 schrieb Christoph Hellwig: On Thu, Jun 24, 2021 at 10:07:14AM +0200, Christian König wrote: The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them. Which is another goddamn layering violation of a subsystem that has no business at all poking into the scatterlist structure, yes. Completely agree, but it is also the easiest way to get away from the scatterlist as trasnport vehicle for the dma_addresses. [SNIP] My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month. Awesome! I have a bit of related work on the DMA mapping subsystems, so let's sync up as soon as you have some first sketches. Don't start cheering to fast. I've already converted a bunch of the GPU drivers, but there are at least 6 GPU still needing to be fixed and on top of that comes VA-API and a few others. What are your plans for the DMA mapping subsystem? Btw, one thing I noticed when looking over the dma-buf instances is that there is a lot of duplicated code for creating a sg_table from pages, and then mapping it. It would be good if we could move toward common helpers instead of duplicating that all over again. Can you give an example? Thanks, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Avoid printing of stack contents on firmware load error
Hi Jiri,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc7 next-20210624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jiri-Kosina/drm-amdgpu-Avoid-printing-of-stack-contents-on-firmware-load-error/20210624-173740
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
7426cedc7dad67bf3c71ea6cc29ab7822e1a453f
config: arm64-randconfig-r006-20210622 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
7c8a507272587f181ec29401453949ebcd8fec65)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
#
https://github.com/0day-ci/linux/commit/f9d4f2041c2724ff3c9126761199d37acede1187
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jiri-Kosina/drm-amdgpu-Avoid-printing-of-stack-contents-on-firmware-load-error/20210624-173740
git checkout f9d4f2041c2724ff3c9126761199d37acede1187
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/amd/amdgpu/psp_v12_0.c:111:1: warning: unused label 'out'
>> [-Wunused-label]
out:
^~~~
1 warning generated.
vim +/out +111 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 47
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 48 static int
psp_v12_0_init_microcode(struct psp_context *psp)
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 49 {
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 50struct amdgpu_device *adev =
psp->adev;
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 51const char *chip_name;
6627d1c1a82ba7 Changfeng 2020-09-01 52char fw_name[30];
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 53int err = 0;
6627d1c1a82ba7 Changfeng 2020-09-01 54const struct
ta_firmware_header_v1_0 *ta_hdr;
6627d1c1a82ba7 Changfeng 2020-09-01 55DRM_DEBUG("\n");
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 56
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 57switch (adev->asic_type) {
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 58case CHIP_RENOIR:
68697982204b21 Aaron Liu 2020-10-01 59if (adev->apu_flags &
AMD_APU_IS_RENOIR)
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 60chip_name =
"renoir";
68697982204b21 Aaron Liu 2020-10-01 61else
68697982204b21 Aaron Liu 2020-10-01 62chip_name =
"green_sardine";
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 63break;
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 64default:
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 65BUG();
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 66}
6a7a0bdbfa0c24 Aaron Liu 2019-08-09 67
f4503f9eb3a16c Hawking Zhang 2020-04-20 68err =
psp_init_asd_microcode(psp, chip_name);
6627d1c1a82ba7 Changfeng 2020-09-01 69if (err)
f9d4f2041c2724 Jiri Kosina 2021-06-24 70return err;
6627d1c1a82ba7 Changfeng 2020-09-01 71
6627d1c1a82ba7 Changfeng 2020-09-01 72snprintf(fw_name,
sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
6627d1c1a82ba7 Changfeng 2020-09-01 73err =
request_firmware(>psp.ta_fw, fw_name, adev->dev);
6627d1c1a82ba7 Changfeng 2020-09-01 74if (err) {
6627d1c1a82ba7 Changfeng 2020-09-01 75
release_firmware(adev->psp.ta_fw);
6627d1c1a82ba7 Changfeng 2020-09-01 76adev->psp.ta_fw = NULL;
6627d1c1a82ba7 Changfeng 2020-09-01 77dev_info(adev->dev,
6627d1c1a82ba7 Changfeng 2020-09-01 78 "psp v12.0:
Failed to load firmware \"%s\"\n",
6627d1c1a82ba7 Changfeng 2020-09-01 79 fw_name);
6627d1c1a82ba7 Changfeng 2020-09-01 80} else {
6627d1c1a82ba7 Changfeng 2020-09-01 81err =
amdgpu_ucode_validate(adev->psp.ta_fw);
6627d1c1a82ba7 Changfeng 2020-09-01 82if (err)
6627d1c1a82ba7 Changfeng 2020-09-01 83goto out2;
6627d1c1a82ba7 Changfeng 2020-09-01 84
6627d1c1a82ba7 Changfeng 2020-09-01 85ta_hdr = (const struct
ta_firmware_header_v1_0 *)
6627d1c1a82ba7 Changfeng 2020-09-01 86
adev->psp.ta_fw-
Re: [PATCH] drm/amdgpu:use kvcalloc instead of kvmalloc_array
Am 23.06.21 um 11:12 schrieb huqiqiao: kvmalloc_array + __GFP_ZERO is the same with kvcalloc. Signed-off-by: huqiqiao --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9acee4a5b2ba..50edc73525b0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -908,9 +908,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev, unsigned num_entries; num_entries = amdgpu_vm_num_entries(adev, cursor->level); - entry->entries = kvmalloc_array(num_entries, - sizeof(*entry->entries), - GFP_KERNEL | __GFP_ZERO); + entry->entries = kvcalloc(num_entries, + sizeof(*entry->entries), GFP_KERNEL); Sounds like a good idea in general, but the indentation on the second line seems to be of. Christian. if (!entry->entries) return -ENOMEM; } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amdgpu: Restore msix after FLR
Am 24.06.21 um 07:31 schrieb Peng Ju Zhou:
From: "Emily.Deng"
After FLR, the msix will be cleared, so need to re-enable it.
Signed-off-by: Emily.Deng
Signed-off-by: Peng Ju Zhou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 90f50561b43a..ba9edafd4fc6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -277,6 +277,18 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev)
return true;
}
+void amdgpu_restore_msix(struct amdgpu_device *adev)
+{
+#ifdef PCI_IRQ_MSIX
Please drop that ifdef, the macro is always defined upstream.
Christian.
+ u16 ctrl;
+
+ pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS,
);
+ ctrl &= ~PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+ ctrl |= PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+#endif
+}
/**
* amdgpu_irq_init - initialize interrupt handling
*
@@ -558,6 +570,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct
amdgpu_device *adev)
{
int i, j, k;
+ amdgpu_restore_msix(adev);
for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) {
if (!adev->irq.client[i].sources)
continue;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 14:36 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thierry Reding wrote:
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
Hi
Am 24.06.21 um 10:51 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
- if (!dev->irq_enabled)
- return -EOPNOTSUPP;
+#if defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
Or how about:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if defined(CONFIG_DRM_LEGACY)
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
#endif
return drm_dev_has_vblank(dev);
}
?
It's inline, but still readable.
It's definitely better than the original, but it's unclear to me why
you'd prefer this over option 2) below. I guess the only reason I can
think of is emphasizing the conditional compilation. However,
IS_ENABLED() is widely used in this manner specifically to avoid inline
#if, and the compiler optimizes it away.
It's simply more readable to me as the condition is simpler. But option 2 is
also ok.
Perhaps do something like this, then:
if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
}
return drm_dev_has_vblank(dev);
That's about just as readable as the variant involving the preprocessor
but has all the benefits of not using the preprocessor.
Looks like a winner to me. :)
That's the most readable.
But I just remembered that irq_enabled will likely become legacy-only in
the device structure. We'll need an ifdef variant then. :/
Best regards
Thomas
BR,
Jani.
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thierry Reding wrote:
> On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 24.06.21 um 10:51 schrieb Jani Nikula:
>> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>> > > Hi
>> > >
>> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
>> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
>> > > > > b/drivers/gpu/drm/drm_vblank.c
>> > > > > index 3417e1ac7918..10fe16bafcb6 100644
>> > > > > --- a/drivers/gpu/drm/drm_vblank.c
>> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
>> > > > > @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device
>> > > > > *dev, void *data,
>> > > > > unsigned int pipe_index;
>> > > > > unsigned int flags, pipe, high_pipe;
>> > > > > -if (!dev->irq_enabled)
>> > > > > -return -EOPNOTSUPP;
>> > > > > +#if defined(CONFIG_DRM_LEGACY)
>> > > > > +if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
>> > > > > +if (!dev->irq_enabled)
>> > > > > +return -EOPNOTSUPP;
>> > > > > +} else /* if DRIVER_MODESET */
>> > > > > +#endif
>> > > > > +{
>> > > > > +if (!drm_dev_has_vblank(dev))
>> > > > > +return -EOPNOTSUPP;
>> > > > > +}
>> > > >
>> > > > Sheesh I hate this kind of inline #ifdefs.
>> > > >
>> > > > Two alternate suggestions that I believe should be as just efficient:
>> > >
>> > > Or how about:
>> > >
>> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
>> > >
>> > > {
>> > >
>> > > if defined(CONFIG_DRM_LEGACY)
>> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> > >
>> > > return dev->irq_enabled;
>> > >
>> > > #endif
>> > > return drm_dev_has_vblank(dev);
>> > >
>> > > }
>> > >
>> > >
>> > > ?
>> > >
>> > > It's inline, but still readable.
>> >
>> > It's definitely better than the original, but it's unclear to me why
>> > you'd prefer this over option 2) below. I guess the only reason I can
>> > think of is emphasizing the conditional compilation. However,
>> > IS_ENABLED() is widely used in this manner specifically to avoid inline
>> > #if, and the compiler optimizes it away.
>>
>> It's simply more readable to me as the condition is simpler. But option 2 is
>> also ok.
>
> Perhaps do something like this, then:
>
> if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> return dev->irq_enabled;
> }
>
> return drm_dev_has_vblank(dev);
>
> That's about just as readable as the variant involving the preprocessor
> but has all the benefits of not using the preprocessor.
Looks like a winner to me. :)
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amdgpu: add function to show ucode name via id
On Thu, Jun 24, 2021 at 02:47:22PM +0800, Yu, Lang wrote:
> From: Lang Yu
>
> Implement function amdgpu_ucode_show to show ucode name
> via ucode id.
>
> Signed-off-by: Lang Yu
Series are Reviewed-by: Huang Rui
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 78 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 2 +
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 2834981f8c08..6a03abb009ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -416,6 +416,84 @@ amdgpu_ucode_get_load_type(struct amdgpu_device *adev,
> int load_type)
> return AMDGPU_FW_LOAD_DIRECT;
> }
>
> +const char *amdgpu_ucode_show(enum AMDGPU_UCODE_ID ucode_id)
> +{
> + switch (ucode_id) {
> + case AMDGPU_UCODE_ID_SDMA0:
> + return "SDMA0";
> + case AMDGPU_UCODE_ID_SDMA1:
> + return "SDMA1";
> + case AMDGPU_UCODE_ID_SDMA2:
> + return "SDMA2";
> + case AMDGPU_UCODE_ID_SDMA3:
> + return "SDMA3";
> + case AMDGPU_UCODE_ID_SDMA4:
> + return "SDMA4";
> + case AMDGPU_UCODE_ID_SDMA5:
> + return "SDMA5";
> + case AMDGPU_UCODE_ID_SDMA6:
> + return "SDMA6";
> + case AMDGPU_UCODE_ID_SDMA7:
> + return "SDMA7";
> + case AMDGPU_UCODE_ID_CP_CE:
> + return "CP_CE";
> + case AMDGPU_UCODE_ID_CP_PFP:
> + return "CP_PFP";
> + case AMDGPU_UCODE_ID_CP_ME:
> + return "CP_ME";
> + case AMDGPU_UCODE_ID_CP_MEC1:
> + return "CP_MEC1";
> + case AMDGPU_UCODE_ID_CP_MEC1_JT:
> + return "CP_MEC1_JT";
> + case AMDGPU_UCODE_ID_CP_MEC2:
> + return "CP_MEC2";
> + case AMDGPU_UCODE_ID_CP_MEC2_JT:
> + return "CP_MEC2_JT";
> + case AMDGPU_UCODE_ID_CP_MES:
> + return "CP_MES";
> + case AMDGPU_UCODE_ID_CP_MES_DATA:
> + return "CP_MES_DATA";
> + case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_CNTL:
> + return "RLC_RESTORE_LIST_CNTL";
> + case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_GPM_MEM:
> + return "RLC_RESTORE_LIST_GPM_MEM";
> + case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_SRM_MEM:
> + return "RLC_RESTORE_LIST_SRM_MEM";
> + case AMDGPU_UCODE_ID_RLC_IRAM:
> + return "RLC_IRAM";
> + case AMDGPU_UCODE_ID_RLC_DRAM:
> + return "RLC_DRAM";
> + case AMDGPU_UCODE_ID_RLC_G:
> + return "RLC_G";
> + case AMDGPU_UCODE_ID_STORAGE:
> + return "STORAGE";
> + case AMDGPU_UCODE_ID_SMC:
> + return "SMC";
> + case AMDGPU_UCODE_ID_UVD:
> + return "UVD";
> + case AMDGPU_UCODE_ID_UVD1:
> + return "UVD1";
> + case AMDGPU_UCODE_ID_VCE:
> + return "VCE";
> + case AMDGPU_UCODE_ID_VCN:
> + return "VCN";
> + case AMDGPU_UCODE_ID_VCN1:
> + return "VCN1";
> + case AMDGPU_UCODE_ID_DMCU_ERAM:
> + return "DMCU_ERAM";
> + case AMDGPU_UCODE_ID_DMCU_INTV:
> + return "DMCU_INTV";
> + case AMDGPU_UCODE_ID_VCN0_RAM:
> + return "VCN0_RAM";
> + case AMDGPU_UCODE_ID_VCN1_RAM:
> + return "VCN1_RAM";
> + case AMDGPU_UCODE_ID_DMCUB:
> + return "DMCUB";
> + default:
> + return "UNKNOWN UCODE";
> + }
> +}
> +
> #define FW_VERSION_ATTR(name, mode, field) \
> static ssize_t show_##name(struct device *dev,
> \
> struct device_attribute *attr,\
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 270309e7f5f5..4b0d34f1d450 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -449,4 +449,6 @@ void amdgpu_ucode_sysfs_fini(struct amdgpu_device *adev);
> enum amdgpu_firmware_load_type
> amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int load_type);
>
> +const char *amdgpu_ucode_show(enum AMDGPU_UCODE_ID ucode_id);
> +
> #endif
> --
> 2.25.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
Am 24.06.21 um 07:34 schrieb Christoph Hellwig: On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: I understand the argument and I agree that for the generic case, the top of the stack can't assume anything. Having said that, in this case the SGL is encapsulated inside a dma-buf object. But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable. The key point is that accessing the underlying pages even when DMA-bufs are backed by system memory is illegal. Daniel even created a patch which mangles the page pointers in sg_tables used by DMA-buf to make sure that people don't try to use them. So the conclusion is that using sg_table in the DMA-buf framework was just the wrong data structure and we should have invented a new one. But then people would have complained that we have a duplicated infrastructure (which is essentially true). My best plan to get out of this mess is that we change the DMA-buf interface to use an array of dma_addresses instead of the sg_table object and I have already been working on this actively the last few month. Regards, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 21/27] drm/tegra: Don't set struct drm_device.irq_enabled
On Thu, Jun 24, 2021 at 09:29:10AM +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in tegra. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Laurent Pinchart > Acked-by: Daniel Vetter > --- > drivers/gpu/drm/tegra/drm.c | 7 --- > 1 file changed, 7 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:51 schrieb Jani Nikula:
> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 3417e1ac7918..10fe16bafcb6 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device
> > > > > *dev, void *data,
> > > > > unsigned int pipe_index;
> > > > > unsigned int flags, pipe, high_pipe;
> > > > > - if (!dev->irq_enabled)
> > > > > - return -EOPNOTSUPP;
> > > > > +#if defined(CONFIG_DRM_LEGACY)
> > > > > + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> > > > > + if (!dev->irq_enabled)
> > > > > + return -EOPNOTSUPP;
> > > > > + } else /* if DRIVER_MODESET */
> > > > > +#endif
> > > > > + {
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > + return -EOPNOTSUPP;
> > > > > + }
> > > >
> > > > Sheesh I hate this kind of inline #ifdefs.
> > > >
> > > > Two alternate suggestions that I believe should be as just efficient:
> > >
> > > Or how about:
> > >
> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > >
> > > {
> > >
> > > if defined(CONFIG_DRM_LEGACY)
> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > >
> > > return dev->irq_enabled;
> > >
> > > #endif
> > > return drm_dev_has_vblank(dev);
> > >
> > > }
> > >
> > >
> > > ?
> > >
> > > It's inline, but still readable.
> >
> > It's definitely better than the original, but it's unclear to me why
> > you'd prefer this over option 2) below. I guess the only reason I can
> > think of is emphasizing the conditional compilation. However,
> > IS_ENABLED() is widely used in this manner specifically to avoid inline
> > #if, and the compiler optimizes it away.
>
> It's simply more readable to me as the condition is simpler. But option 2 is
> also ok.
Perhaps do something like this, then:
if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
}
return drm_dev_has_vblank(dev);
That's about just as readable as the variant involving the preprocessor
but has all the benefits of not using the preprocessor.
Thierry
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Fix resource leak on probe error path
From: Jiri Kosina
This reverts commit 4192f7b5768912ceda82be2f83c87ea7181f9980.
It is not true (as stated in the reverted commit changelog) that we never
unmap the BAR on failure; it actually does happen properly on
amdgpu_driver_load_kms() -> amdgpu_driver_unload_kms() ->
amdgpu_device_fini() error path.
What's worse, this commit actually completely breaks resource freeing on
probe failure (like e.g. failure to load microcode), as
amdgpu_driver_unload_kms() notices adev->rmmio being NULL and bails too
early, leaving all the resources that'd normally be freed in
amdgpu_acpi_fini() and amdgpu_device_fini() still hanging around, leading
to all sorts of oopses when someone tries to, for example, access the
sysfs and procfs resources which are still around while the driver is
gone.
Fixes: 4192f7b57689 ("drm/amdgpu: unmap register bar on device init failure")
Reported-by: Vojtech Pavlik
Signed-off-by: Jiri Kosina
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 57ec108b5972..0f1c0e17a587 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3414,13 +3414,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
r = amdgpu_device_get_job_timeout_settings(adev);
if (r) {
dev_err(adev->dev, "invalid lockup_timeout parameter syntax\n");
- goto failed_unmap;
+ return r;
}
/* early init functions */
r = amdgpu_device_ip_early_init(adev);
if (r)
- goto failed_unmap;
+ return r;
/* doorbell bar mapping and doorbell index init*/
amdgpu_device_doorbell_init(adev);
@@ -3646,10 +3646,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
failed:
amdgpu_vf_error_trans_all(adev);
-failed_unmap:
- iounmap(adev->rmmio);
- adev->rmmio = NULL;
-
return r;
}
--
2.12.3
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amdgpu: Avoid printing of stack contents on firmware load error
From: Jiri Kosina
In case when psp_init_asd_microcode() fails to load ASD microcode file,
psp_v12_0_init_microcode() tries to print the firmware filename that
failed to load before bailing out.
This is wrong because:
- the firmware filename it would want it print is an incorrect one as
psp_init_asd_microcode() and psp_v12_0_init_microcode() are loading
different filenames
- it tries to print fw_name, but that's not yet been initialized by that
time, so it prints random stack contents, e.g.
amdgpu :04:00.0: Direct firmware load for amdgpu/renoir_asd.bin failed
with error -2
amdgpu :04:00.0: amdgpu: fail to initialize asd microcode
amdgpu :04:00.0: amdgpu: psp v12.0: Failed to load firmware
"\xfeTO\x8e\xff\xff"
Fix that by bailing out immediately, instead of priting the bogus error
message.
Reported-by: Vojtech Pavlik
Signed-off-by: Jiri Kosina
---
v1 -> v2: remove now-unused label
drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index c4828bd3264b..b0ee77ee80b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -67,7 +67,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
err = psp_init_asd_microcode(psp, chip_name);
if (err)
- goto out;
+ return err;
snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
err = request_firmware(>psp.ta_fw, fw_name, adev->dev);
@@ -80,7 +80,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
} else {
err = amdgpu_ucode_validate(adev->psp.ta_fw);
if (err)
- goto out2;
+ goto out;
ta_hdr = (const struct ta_firmware_header_v1_0 *)
adev->psp.ta_fw->data;
@@ -105,10 +105,9 @@ static int psp_v12_0_init_microcode(struct psp_context
*psp)
return 0;
-out2:
+out:
release_firmware(adev->psp.ta_fw);
adev->psp.ta_fw = NULL;
-out:
if (err) {
dev_err(adev->dev,
"psp v12.0: Failed to load firmware \"%s\"\n",
--
2.12.3
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: AMDGPU error: "[drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out!"
On 2021-06-04 3:08 p.m., Michel Dänzer wrote: > On 2021-06-04 2:33 p.m., Alex Deucher wrote: >> On Fri, Jun 4, 2021 at 3:47 AM Michel Dänzer wrote: >>> >>> On 2021-05-19 3:57 p.m., Alex Deucher wrote: On Wed, May 19, 2021 at 4:48 AM Michel Dänzer wrote: > > On 2021-05-19 12:05 a.m., Alex Deucher wrote: >> On Tue, May 18, 2021 at 10:11 AM Michel Dänzer >> wrote: >>> >>> On 2021-05-17 11:33 a.m., xgqt wrote: Hello! I run a AMD laptop "81NC Lenovo IdeaPad S340-15API" - AMD Ryzen 5 3500U with Radeon Vega 8 Graphics. Recently some breakages started happening for me. In about 1h after boot-up while using a KDE desktop machine GUI would freeze. Sometimes it would be possible to move the mouse but the rest will be frozen. Screen may start blinking or go black. I'm not sure if this is my kernel, firmware or the hardware. I don't understands dmesg that's why I'm guessing, but I think it is the firmware since this behavior started around 2021-05-15. From my Portage logs I see that I updated my firmware on 2021-05-14 at 18:16:06. So breakages started with my kernel: 5.10.27 and FW: 20210511. After breakage I jumped to a older kernel 5.4.97 and compiled 5.12.4. I didn't notice a breakage on 5.4.97 but system ran ~40 minutes. So I booted to newly compiled 5.12.4 where I was ~1h and it broke. After that I booted to 5.4.97 again and downgraded my FW. While I'm writing this I'm booted to kernel: 5.12.4 with FW: 20210315. I also described my situation on the Gentoo bugzilla: https://bugs.gentoo.org/790566 "dmesg.log" attached here is from the time machine run fine (at the moment); "errors_sat_may_15_072825_pm_cest_2021.log" is a dmesg log from the time system broke Can I get any help with this? What are the next steps I should take? Any other files I should provide? >>> >>> I've hit similar hangs with a Lenovo ThinkPad E595 (Ryzen 7 3700U / >>> Picasso / RAVEN 0x1002:0x15D8 0x17AA:0x5124 0xC1). I'm also suspecting >>> them to be firware related. The hangs occurred with firmware from the >>> AMD 20.50 release. I'm currently running with firmware from the 20.40 >>> release, no hang in almost 2 weeks (the hangs happened within 1-2 days >>> after boot). >> >> Can you narrow down which firmware(s) cause the problem? > > I'll try, but note I'm not really sure yet my hangs were related to > firmware (only). Anyway, I'll try narrowing it down. Thanks. Does this patch help? https://patchwork.freedesktop.org/patch/433701/ >>> >>> Unfortunately not. After no hangs for two weeks with older firmware, I just >>> got a hang again within a day with newer firmware and a kernel with this >>> fix. >>> >>> >>> I'll try and narrow down which firmware triggers it now. Does Picasso use >>> the picasso_*.bin ones only, or others as well? >> >> The picasso ones and raven_dmcu.bin. > > Thanks. raven_dmcu.bin hasn't changed, so I'm trying to bisect the 8 Picasso > ones which have changed: > > picasso_asd.bin > picasso_ce.bin > picasso_me.bin > picasso_mec2.bin > picasso_mec.bin > picasso_pfp.bin > picasso_sdma.bin > picasso_vcn.bin Things are pointing to picasso_sdma.bin. I'm currently running with only that one reverted to linux-firmware 20210315, and haven't got any hangs for a week. Note that I've previously gone for a week without a hang even with firmware which had hung before. So there's still a small chance that I'm just on another lucky run. That said, Pierre-Eric has also homed in on raven_sdma.bin for similar hangs, and reverting to older firmware seems to have helped multiple people on bug reports. So, I think it makes sense for you guys to start looking for what could be going wrong with the Picasso/Raven SDMA firmware from 20.50. One thing I noticed is that the SDMA firmware from 20.50 advertises the same feature version, but a *lower* firmware version than the one from 18.50. So it might be worth double-checking that there wasn't an accidental downgrade to some older version. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] drm/amdgpu: add function to show psp_gfx_cmd name via id
From: Lang Yu
Implement function psp_gfx_cmd_show to show cmd name
via cmd id.
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 38 +
1 file changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 40da29d8ec1e..ff26185b1485 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -353,6 +353,44 @@ int psp_wait_for(struct psp_context *psp, uint32_t
reg_index,
return -ETIME;
}
+static const char *psp_gfx_cmd_show(enum psp_gfx_cmd_id cmd_id)
+{
+ switch (cmd_id) {
+ case GFX_CMD_ID_LOAD_TA:
+ return "LOAD_TA";
+ case GFX_CMD_ID_UNLOAD_TA:
+ return "UNLOAD_TA";
+ case GFX_CMD_ID_INVOKE_CMD:
+ return "INVOKE_CMD";
+ case GFX_CMD_ID_LOAD_ASD:
+ return "LOAD_ASD";
+ case GFX_CMD_ID_SETUP_TMR:
+ return "SETUP_TMR";
+ case GFX_CMD_ID_LOAD_IP_FW:
+ return "LOAD_IP_FW";
+ case GFX_CMD_ID_DESTROY_TMR:
+ return "DESTROY_TMR";
+ case GFX_CMD_ID_SAVE_RESTORE:
+ return "SAVE_RESTORE_IP_FW";
+ case GFX_CMD_ID_SETUP_VMR:
+ return "SETUP_VMR";
+ case GFX_CMD_ID_DESTROY_VMR:
+ return "DESTROY_VMR";
+ case GFX_CMD_ID_PROG_REG:
+ return "PROG_REG";
+ case GFX_CMD_ID_GET_FW_ATTESTATION:
+ return "GET_FW_ATTESTATION";
+ case GFX_CMD_ID_LOAD_TOC:
+ return "ID_LOAD_TOC";
+ case GFX_CMD_ID_AUTOLOAD_RLC:
+ return "AUTOLOAD_RLC";
+ case GFX_CMD_ID_BOOT_CFG:
+ return "BOOT_CFG";
+ default:
+ return "UNKNOWN CMD";
+ }
+}
+
static int
psp_cmd_submit_buf(struct psp_context *psp,
struct amdgpu_firmware_info *ucode,
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/3] drm/amdgpu: show explicit name instead of id in psp_cmd_submit_buf
From: Lang Yu
Use amdgpu_ucode_show to show ucode name and psp_gfx_cmd_show to
show psp_gfx_cmd name in psp_cmd_submit_buf.
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index ff26185b1485..76faf10bcd54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -450,10 +450,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
*/
if (!skip_unsupport && (psp->cmd_buf_mem->resp.status || !timeout) &&
!ras_intr) {
if (ucode)
- DRM_WARN("failed to load ucode id (%d) ",
- ucode->ucode_id);
- DRM_WARN("psp command (0x%X) failed and response status is
(0x%X)\n",
-psp->cmd_buf_mem->cmd_id,
+ DRM_WARN("failed to load ucode (%s) ",
+ amdgpu_ucode_show(ucode->ucode_id));
+ DRM_WARN("psp gfx command (%s) failed and response status is
(0x%X)\n",
+psp_gfx_cmd_show(psp->cmd_buf_mem->cmd_id),
psp->cmd_buf_mem->resp.status);
if (!timeout) {
mutex_unlock(>mutex);
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/3] drm/amdgpu: add function to show ucode name via id
From: Lang Yu
Implement function amdgpu_ucode_show to show ucode name
via ucode id.
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 78 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 2 +
2 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 2834981f8c08..6a03abb009ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -416,6 +416,84 @@ amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int
load_type)
return AMDGPU_FW_LOAD_DIRECT;
}
+const char *amdgpu_ucode_show(enum AMDGPU_UCODE_ID ucode_id)
+{
+ switch (ucode_id) {
+ case AMDGPU_UCODE_ID_SDMA0:
+ return "SDMA0";
+ case AMDGPU_UCODE_ID_SDMA1:
+ return "SDMA1";
+ case AMDGPU_UCODE_ID_SDMA2:
+ return "SDMA2";
+ case AMDGPU_UCODE_ID_SDMA3:
+ return "SDMA3";
+ case AMDGPU_UCODE_ID_SDMA4:
+ return "SDMA4";
+ case AMDGPU_UCODE_ID_SDMA5:
+ return "SDMA5";
+ case AMDGPU_UCODE_ID_SDMA6:
+ return "SDMA6";
+ case AMDGPU_UCODE_ID_SDMA7:
+ return "SDMA7";
+ case AMDGPU_UCODE_ID_CP_CE:
+ return "CP_CE";
+ case AMDGPU_UCODE_ID_CP_PFP:
+ return "CP_PFP";
+ case AMDGPU_UCODE_ID_CP_ME:
+ return "CP_ME";
+ case AMDGPU_UCODE_ID_CP_MEC1:
+ return "CP_MEC1";
+ case AMDGPU_UCODE_ID_CP_MEC1_JT:
+ return "CP_MEC1_JT";
+ case AMDGPU_UCODE_ID_CP_MEC2:
+ return "CP_MEC2";
+ case AMDGPU_UCODE_ID_CP_MEC2_JT:
+ return "CP_MEC2_JT";
+ case AMDGPU_UCODE_ID_CP_MES:
+ return "CP_MES";
+ case AMDGPU_UCODE_ID_CP_MES_DATA:
+ return "CP_MES_DATA";
+ case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_CNTL:
+ return "RLC_RESTORE_LIST_CNTL";
+ case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_GPM_MEM:
+ return "RLC_RESTORE_LIST_GPM_MEM";
+ case AMDGPU_UCODE_ID_RLC_RESTORE_LIST_SRM_MEM:
+ return "RLC_RESTORE_LIST_SRM_MEM";
+ case AMDGPU_UCODE_ID_RLC_IRAM:
+ return "RLC_IRAM";
+ case AMDGPU_UCODE_ID_RLC_DRAM:
+ return "RLC_DRAM";
+ case AMDGPU_UCODE_ID_RLC_G:
+ return "RLC_G";
+ case AMDGPU_UCODE_ID_STORAGE:
+ return "STORAGE";
+ case AMDGPU_UCODE_ID_SMC:
+ return "SMC";
+ case AMDGPU_UCODE_ID_UVD:
+ return "UVD";
+ case AMDGPU_UCODE_ID_UVD1:
+ return "UVD1";
+ case AMDGPU_UCODE_ID_VCE:
+ return "VCE";
+ case AMDGPU_UCODE_ID_VCN:
+ return "VCN";
+ case AMDGPU_UCODE_ID_VCN1:
+ return "VCN1";
+ case AMDGPU_UCODE_ID_DMCU_ERAM:
+ return "DMCU_ERAM";
+ case AMDGPU_UCODE_ID_DMCU_INTV:
+ return "DMCU_INTV";
+ case AMDGPU_UCODE_ID_VCN0_RAM:
+ return "VCN0_RAM";
+ case AMDGPU_UCODE_ID_VCN1_RAM:
+ return "VCN1_RAM";
+ case AMDGPU_UCODE_ID_DMCUB:
+ return "DMCUB";
+ default:
+ return "UNKNOWN UCODE";
+ }
+}
+
#define FW_VERSION_ATTR(name, mode, field) \
static ssize_t show_##name(struct device *dev, \
struct device_attribute *attr,\
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 270309e7f5f5..4b0d34f1d450 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -449,4 +449,6 @@ void amdgpu_ucode_sysfs_fini(struct amdgpu_device *adev);
enum amdgpu_firmware_load_type
amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int load_type);
+const char *amdgpu_ucode_show(enum AMDGPU_UCODE_ID ucode_id);
+
#endif
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
Hi Thomas,
Thank you for the patch.
On Thu, Jun 24, 2021 at 09:29:13AM +0200, Thomas Zimmermann wrote:
> The field drm_device.irq_enabled is only used by legacy drivers
> with userspace modesetting. Don't set it in vkms.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
> ---
> drivers/gpu/drm/vkms/vkms_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 027ffe759440..496de38ad983 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
> goto out_devres;
> }
>
> - vkms_device->drm.irq_enabled = true;
> -
> ret = drm_vblank_init(_device->drm, 1);
> if (ret) {
> DRM_ERROR("Failed to vblank\n");
--
Regards,
Laurent Pinchart
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 16/27] drm/rcar-du: Don't set struct drm_device.irq_enabled
Hi Thomas, Thank you for the patch. On Thu, Jun 24, 2021 at 09:29:05AM +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in rcar-du. > > Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index bfbff90588cb..e289a66594a7 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -593,8 +593,6 @@ static int rcar_du_probe(struct platform_device *pdev) > goto error; > } > > - rcdu->ddev.irq_enabled = 1; > - > /* >* Register the DRM device with the core and the connectors with >* sysfs. -- Regards, Laurent Pinchart ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:51 schrieb Jani Nikula:
> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 3417e1ac7918..10fe16bafcb6 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device
> > > > > *dev, void *data,
> > > > > unsigned int pipe_index;
> > > > > unsigned int flags, pipe, high_pipe;
> > > > > - if (!dev->irq_enabled)
> > > > > - return -EOPNOTSUPP;
> > > > > +#if defined(CONFIG_DRM_LEGACY)
> > > > > + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> > > > > + if (!dev->irq_enabled)
> > > > > + return -EOPNOTSUPP;
> > > > > + } else /* if DRIVER_MODESET */
> > > > > +#endif
> > > > > + {
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > + return -EOPNOTSUPP;
> > > > > + }
> > > >
> > > > Sheesh I hate this kind of inline #ifdefs.
> > > >
> > > > Two alternate suggestions that I believe should be as just efficient:
> > >
> > > Or how about:
> > >
> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > >
> > > {
> > >
> > > if defined(CONFIG_DRM_LEGACY)
> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > >
> > > return dev->irq_enabled;
> > >
> > > #endif
> > > return drm_dev_has_vblank(dev);
> > >
> > > }
> > >
> > >
> > > ?
> > >
> > > It's inline, but still readable.
> >
> > It's definitely better than the original, but it's unclear to me why
> > you'd prefer this over option 2) below. I guess the only reason I can
> > think of is emphasizing the conditional compilation. However,
> > IS_ENABLED() is widely used in this manner specifically to avoid inline
> > #if, and the compiler optimizes it away.
>
> It's simply more readable to me as the condition is simpler. But option 2 is
> also ok.
Either option looks good to me.
Reviewed-by: Liviu Dudau
Thanks for doing that!
Liviu
>
> Best regards
> Thomas
>
> >
> > BR,
> > Jani.
> >
> >
> > >
> > > Best regards
> > > Thomas
> > >
> > > >
> > > > 1) The more verbose:
> > > >
> > > > #if defined(CONFIG_DRM_LEGACY)
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #else
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #endif
> > > >
> > > > 2) The more compact:
> > > >
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
> > > > unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > >
> > > > Then, in drm_wait_vblank_ioctl().
> > > >
> > > > if (!drm_wait_vblank_supported(dev))
> > > > return -EOPNOTSUPP;
> > > >
> > > > The compiler should do the right thing without any explicit inline
> > > > keywords etc.
> > > >
> > > > BR,
> > > > Jani.
> > > >
> > > > > if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> > > > > return -EINVAL;
> > > > > @@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct
> > > > > drm_device *dev, void *data,
> > > > > if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > > > > return -EOPNOTSUPP;
> > > > > - if (!dev->irq_enabled)
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > return -EOPNOTSUPP;
> > > > > crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
> > > > > @@ -2082,7 +2090,7 @@ int drm_crtc_queue_sequence_ioctl(struct
> > > > > drm_device *dev, void *data,
> > > > > if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > > > > return -EOPNOTSUPP;
> > > > > - if (!dev->irq_enabled)
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > return -EOPNOTSUPP;
> > > > > crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
> > > >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
[PATCH] drm/amdgpu: Avoid printing of stack contents on firmware load error
From: Jiri Kosina In case when psp_init_asd_microcode() fails to load ASD microcode file, psp_v12_0_init_microcode() tries to print the firmware filename that failed to load before bailing out. This is wrong because: - the firmware filename it would want it print is an incorrect one as psp_init_asd_microcode() and psp_v12_0_init_microcode() are loading different filenames - it tries to print fw_name, but that's not yet been initialized by that time, so it prints random stack contents, e.g. amdgpu :04:00.0: Direct firmware load for amdgpu/renoir_asd.bin failed with error -2 amdgpu :04:00.0: amdgpu: fail to initialize asd microcode amdgpu :04:00.0: amdgpu: psp v12.0: Failed to load firmware "\xfeTO\x8e\xff\xff" Fix that by bailing out immediately, instead of priting the bogus error message. Reported-by: Vojtech Pavlik Signed-off-by: Jiri Kosina --- drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c index c4828bd3264b..5b21e22ad4b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c @@ -67,7 +67,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp) err = psp_init_asd_microcode(psp, chip_name); if (err) - goto out; + return err; snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name); err = request_firmware(>psp.ta_fw, fw_name, adev->dev); -- 2.12.3 -- Jiri Kosina SUSE Labs ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amdgpu: Restore msix after FLR
From: "Emily.Deng"
After FLR, the msix will be cleared, so need to re-enable it.
Signed-off-by: Emily.Deng
Signed-off-by: Peng Ju Zhou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 90f50561b43a..ba9edafd4fc6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -277,6 +277,18 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev)
return true;
}
+void amdgpu_restore_msix(struct amdgpu_device *adev)
+{
+#ifdef PCI_IRQ_MSIX
+ u16 ctrl;
+
+ pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS,
);
+ ctrl &= ~PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+ ctrl |= PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+#endif
+}
/**
* amdgpu_irq_init - initialize interrupt handling
*
@@ -558,6 +570,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct
amdgpu_device *adev)
{
int i, j, k;
+ amdgpu_restore_msix(adev);
for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) {
if (!adev->irq.client[i].sources)
continue;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:51 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
- if (!dev->irq_enabled)
- return -EOPNOTSUPP;
+#if defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
Or how about:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if defined(CONFIG_DRM_LEGACY)
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
#endif
return drm_dev_has_vblank(dev);
}
?
It's inline, but still readable.
It's definitely better than the original, but it's unclear to me why
you'd prefer this over option 2) below. I guess the only reason I can
think of is emphasizing the conditional compilation. However,
IS_ENABLED() is widely used in this manner specifically to avoid inline
#if, and the compiler optimizes it away.
It's simply more readable to me as the condition is simpler. But option
2 is also ok.
Best regards
Thomas
BR,
Jani.
Best regards
Thomas
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- if (!dev->irq_enabled)
+ if (!drm_dev_has_vblank(dev))
return -EOPNOTSUPP;
crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
@@ -2082,7 +2090,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- if (!dev->irq_enabled)
+ if (!drm_dev_has_vblank(dev))
return -EOPNOTSUPP;
crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:06 schrieb Jani Nikula:
>> On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>>> index 3417e1ac7918..10fe16bafcb6 100644
>>> --- a/drivers/gpu/drm/drm_vblank.c
>>> +++ b/drivers/gpu/drm/drm_vblank.c
>>> @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev,
>>> void *data,
>>> unsigned int pipe_index;
>>> unsigned int flags, pipe, high_pipe;
>>>
>>> - if (!dev->irq_enabled)
>>> - return -EOPNOTSUPP;
>>> +#if defined(CONFIG_DRM_LEGACY)
>>> + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
>>> + if (!dev->irq_enabled)
>>> + return -EOPNOTSUPP;
>>> + } else /* if DRIVER_MODESET */
>>> +#endif
>>> + {
>>> + if (!drm_dev_has_vblank(dev))
>>> + return -EOPNOTSUPP;
>>> + }
>>
>> Sheesh I hate this kind of inline #ifdefs.
>>
>> Two alternate suggestions that I believe should be as just efficient:
>
> Or how about:
>
> static bool drm_wait_vblank_supported(struct drm_device *dev)
>
> {
>
> if defined(CONFIG_DRM_LEGACY)
> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>
> return dev->irq_enabled;
>
> #endif
> return drm_dev_has_vblank(dev);
>
> }
>
>
> ?
>
> It's inline, but still readable.
It's definitely better than the original, but it's unclear to me why
you'd prefer this over option 2) below. I guess the only reason I can
think of is emphasizing the conditional compilation. However,
IS_ENABLED() is widely used in this manner specifically to avoid inline
#if, and the compiler optimizes it away.
BR,
Jani.
>
> Best regards
> Thomas
>
>>
>> 1) The more verbose:
>>
>> #if defined(CONFIG_DRM_LEGACY)
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> return dev->irq_enabled;
>> else
>> return drm_dev_has_vblank(dev);
>> }
>> #else
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> return drm_dev_has_vblank(dev);
>> }
>> #endif
>>
>> 2) The more compact:
>>
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
>> unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> return dev->irq_enabled;
>> else
>> return drm_dev_has_vblank(dev);
>> }
>>
>> Then, in drm_wait_vblank_ioctl().
>>
>> if (!drm_wait_vblank_supported(dev))
>> return -EOPNOTSUPP;
>>
>> The compiler should do the right thing without any explicit inline
>> keywords etc.
>>
>> BR,
>> Jani.
>>
>>>
>>> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
>>> return -EINVAL;
>>> @@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device
>>> *dev, void *data,
>>> if (!drm_core_check_feature(dev, DRIVER_MODESET))
>>> return -EOPNOTSUPP;
>>>
>>> - if (!dev->irq_enabled)
>>> + if (!drm_dev_has_vblank(dev))
>>> return -EOPNOTSUPP;
>>>
>>> crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
>>> @@ -2082,7 +2090,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device
>>> *dev, void *data,
>>> if (!drm_core_check_feature(dev, DRIVER_MODESET))
>>> return -EOPNOTSUPP;
>>>
>>> - if (!dev->irq_enabled)
>>> + if (!drm_dev_has_vblank(dev))
>>> return -EOPNOTSUPP;
>>>
>>> crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
>>
--
Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 11/27] drm/imx: Don't set struct drm_device.irq_enabled
On Thu, 2021-06-24 at 09:29 +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in imx. > > v3: > * move dcss changes into separate patch (Laurentiu) > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Laurent Pinchart > Acked-by: Daniel Vetter > --- > drivers/gpu/drm/imx/imx-drm-core.c | 11 --- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c > b/drivers/gpu/drm/imx/imx-drm-core.c > index 76819a8ac37f..9558e9e1b431 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -207,17 +207,6 @@ static int imx_drm_bind(struct device *dev) > if (IS_ERR(drm)) > return PTR_ERR(drm); > > - /* > - * enable drm irq mode. > - * - with irq_enabled = true, we can use the vblank feature. > - * > - * P.S. note that we wouldn't use drm irq handler but > - * just specific driver own one instead because > - * drm framework supports only one irq handler and > - * drivers can well take care of their interrupts > - */ > - drm->irq_enabled = true; > - > /* >* set max width and height as default value(4096x4096). >* this value would be used to check framebuffer size limitation Acked-by: Philipp Zabel regards Philipp ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state
for non-legacy drivers. For legacy drivers with userspace modesetting,
the original test remains in drm_wait_vblank_ioctl().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _device.dev and set as the device paramter in drm_dev_alloc().
*
- * These IRQ helpers are strictly optional. Drivers which roll their own only
- * need to set _device.irq_enabled to signal the DRM core that vblank
- * interrupts are working. Since these helpers don't automatically clean up the
- * requested interrupt like e.g. devm_request_irq() they're not really
+ * These IRQ helpers are strictly optional. Since these helpers don't
automatically
+ * clean up the requested interrupt like e.g. devm_request_irq() they're not
really
* recommended.
*/
@@ -91,9 +89,7 @@
* and after the installation.
*
* This is the simplified helper interface provided for drivers with no
special
- * needs. Drivers which need to install interrupt handlers for multiple
- * interrupts must instead set _device.irq_enabled to signal the DRM core
- * that vblank interrupts are available.
+ * needs.
*
* @irq must match the interrupt number that would be passed to request_irq(),
* if called directly instead of using this helper function.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used
drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function
fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
- if (!dev->irq_enabled)
- return -EOPNOTSUPP;
+#if defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
Or how about:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if defined(CONFIG_DRM_LEGACY)
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
#endif
return drm_dev_has_vblank(dev);
}
?
It's inline, but still readable.
Best regards
Thomas
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state
for non-legacy drivers. For legacy drivers with userspace modesetting,
the original test remains in drm_wait_vblank_ioctl().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _device.dev and set as the device paramter in drm_dev_alloc().
*
- * These IRQ helpers are strictly optional. Drivers which roll their own only
- * need to set _device.irq_enabled to signal the DRM core that vblank
- * interrupts are working. Since these helpers don't automatically clean up the
- * requested interrupt like e.g. devm_request_irq() they're not really
+ * These IRQ helpers are strictly optional. Since these helpers don't
automatically
+ * clean up the requested interrupt like e.g. devm_request_irq() they're not
really
* recommended.
*/
@@ -91,9 +89,7 @@
* and after the installation.
*
* This is the simplified helper interface provided for drivers with no
special
- * needs. Drivers which need to install interrupt handlers for multiple
- * interrupts must instead set _device.irq_enabled to signal the DRM core
- * that vblank interrupts are available.
+ * needs.
*
* @irq must match the interrupt number that would be passed to request_irq(),
* if called directly instead of using this helper function.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used
drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function
fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
- if (!dev->irq_enabled)
- return -EOPNOTSUPP;
+#if defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
I don't like them either. I guess I'll go with suggestion 2. Thanks for
the feedback.
Best regards
Thomas
Two alternate suggestions that I believe should be as just efficient:
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- if (!dev->irq_enabled)
+ if
Re: [PATCH v3 06/27] drm/i915: Track IRQ state in local device state
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> Replace usage of struct drm_device.irq_enabled with the driver's
> own state field struct drm_i915_private.irq_enabled. The field in
> the DRM device structure is considered legacy and should not be
> used by KMS drivers.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Jani Nikula
and ack for merging through drm-misc-next or whatever you think is best.
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_irq.c | 8
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 01e11fe38642..48c1835bd54b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1134,6 +1134,8 @@ struct drm_i915_private {
> /* For i915gm/i945gm vblank irq workaround */
> u8 vblank_enabled;
>
> + bool irq_enabled;
> +
> /* perform PHY state sanity checks? */
> bool chv_phy_assert[2];
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a11bdb667241..987211f21761 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4488,14 +4488,14 @@ int intel_irq_install(struct drm_i915_private
> *dev_priv)
>*/
> dev_priv->runtime_pm.irqs_enabled = true;
>
> - dev_priv->drm.irq_enabled = true;
> + dev_priv->irq_enabled = true;
>
> intel_irq_reset(dev_priv);
>
> ret = request_irq(irq, intel_irq_handler(dev_priv),
> IRQF_SHARED, DRIVER_NAME, dev_priv);
> if (ret < 0) {
> - dev_priv->drm.irq_enabled = false;
> + dev_priv->irq_enabled = false;
> return ret;
> }
>
> @@ -4521,10 +4521,10 @@ void intel_irq_uninstall(struct drm_i915_private
> *dev_priv)
>* intel_modeset_driver_remove() calling us out of sequence.
>* Would be nice if it didn't do that...
>*/
> - if (!dev_priv->drm.irq_enabled)
> + if (!dev_priv->irq_enabled)
> return;
>
> - dev_priv->drm.irq_enabled = false;
> + dev_priv->irq_enabled = false;
>
> intel_irq_reset(dev_priv);
--
Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
> vblank support. IRQs might be enabled wthout vblanking being supported.
>
> This change also removes the DRM framework's only dependency on IRQ state
> for non-legacy drivers. For legacy drivers with userspace modesetting,
> the original test remains in drm_wait_vblank_ioctl().
>
> v3:
> * optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
> * update docs for drm_irq_uninstall()
> v2:
> * keep the old test for legacy drivers in
> drm_wait_vblank_ioctl() (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Laurent Pinchart
> Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c| 13 -
> drivers/gpu/drm/drm_vblank.c | 16
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c3bd664ea733..945dd82e2ea3 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,10 +74,8 @@
> * only supports devices with a single interrupt on the main device stored in
> * _device.dev and set as the device paramter in drm_dev_alloc().
> *
> - * These IRQ helpers are strictly optional. Drivers which roll their own only
> - * need to set _device.irq_enabled to signal the DRM core that vblank
> - * interrupts are working. Since these helpers don't automatically clean up
> the
> - * requested interrupt like e.g. devm_request_irq() they're not really
> + * These IRQ helpers are strictly optional. Since these helpers don't
> automatically
> + * clean up the requested interrupt like e.g. devm_request_irq() they're not
> really
> * recommended.
> */
>
> @@ -91,9 +89,7 @@
> * and after the installation.
> *
> * This is the simplified helper interface provided for drivers with no
> special
> - * needs. Drivers which need to install interrupt handlers for multiple
> - * interrupts must instead set _device.irq_enabled to signal the DRM core
> - * that vblank interrupts are available.
> + * needs.
> *
> * @irq must match the interrupt number that would be passed to
> request_irq(),
> * if called directly instead of using this helper function.
> @@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
> *
> * Calls the driver's _driver.irq_uninstall function and unregisters the
> IRQ
> * handler. This should only be called by drivers which used
> drm_irq_install()
> - * to set up their interrupt handler. Other drivers must only reset
> - * _device.irq_enabled to false.
> + * to set up their interrupt handler.
> *
> * Note that for kernel modesetting drivers it is a bug if this function
> fails.
> * The sanity checks are only to catch buggy user modesetting drivers which
> call
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac7918..10fe16bafcb6 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
> *data,
> unsigned int pipe_index;
> unsigned int flags, pipe, high_pipe;
>
> - if (!dev->irq_enabled)
> - return -EOPNOTSUPP;
> +#if defined(CONFIG_DRM_LEGACY)
> + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> + if (!dev->irq_enabled)
> + return -EOPNOTSUPP;
> + } else /* if DRIVER_MODESET */
> +#endif
> + {
> + if (!drm_dev_has_vblank(dev))
> + return -EOPNOTSUPP;
> + }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
>
> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> return -EINVAL;
> @@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev,
> void *data,
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EOPNOTSUPP;
>
> - if (!dev->irq_enabled)
> + if (!drm_dev_has_vblank(dev))
>
[PATCH v3 27/27] drm/zte: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in zte. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/zte/zx_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 5506336594e2..064056503ebb 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -75,12 +75,6 @@ static int zx_drm_bind(struct device *dev) goto out_unbind; } - /* -* We will manage irq handler on our own. In this case, irq_enabled -* need to be true for using vblank core support. -*/ - drm->irq_enabled = true; - drm_mode_config_reset(drm); drm_kms_helper_poll_init(drm); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 26/27] drm/xlnx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in xlnx. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index 0c1c50271a88..ac37053412a1 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -111,8 +111,6 @@ static int zynqmp_dpsub_drm_init(struct zynqmp_dpsub *dpsub) if (ret) return ret; - drm->irq_enabled = 1; - drm_kms_helper_poll_init(drm); /* -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 22/27] drm/tidss: Don't use struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't use it in tidss.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/tidss/tidss_irq.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_irq.c
b/drivers/gpu/drm/tidss/tidss_irq.c
index a5ec7931ef6b..2ed3e3296776 100644
--- a/drivers/gpu/drm/tidss/tidss_irq.c
+++ b/drivers/gpu/drm/tidss/tidss_irq.c
@@ -57,9 +57,6 @@ irqreturn_t tidss_irq_handler(int irq, void *arg)
unsigned int id;
dispc_irq_t irqstatus;
- if (WARN_ON(!ddev->irq_enabled))
- return IRQ_NONE;
-
irqstatus = dispc_read_and_clear_irqstatus(tidss->dispc);
for (id = 0; id < tidss->num_crtcs; id++) {
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 25/27] drm/vmwgfx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vmxgfx. All usage of
the field within vmwgfx can safely be removed.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
index b9a9b7ddadbd..4b82f5995452 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
@@ -292,15 +292,11 @@ void vmw_irq_uninstall(struct drm_device *dev)
if (!(dev_priv->capabilities & SVGA_CAP_IRQMASK))
return;
- if (!dev->irq_enabled)
- return;
-
vmw_write(dev_priv, SVGA_REG_IRQMASK, 0);
status = vmw_irq_status_read(dev_priv);
vmw_irq_status_write(dev_priv, status);
- dev->irq_enabled = false;
free_irq(dev->irq, dev);
}
@@ -315,9 +311,6 @@ int vmw_irq_install(struct drm_device *dev, int irq)
{
int ret;
- if (dev->irq_enabled)
- return -EBUSY;
-
vmw_irq_preinstall(dev);
ret = request_threaded_irq(irq, vmw_irq_handler, vmw_thread_fn,
@@ -325,7 +318,6 @@ int vmw_irq_install(struct drm_device *dev, int irq)
if (ret < 0)
return ret;
- dev->irq_enabled = true;
dev->irq = irq;
return ret;
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vkms.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vkms/vkms_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 027ffe759440..496de38ad983 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
goto out_devres;
}
- vkms_device->drm.irq_enabled = true;
-
ret = drm_vblank_init(_device->drm, 1);
if (ret) {
DRM_ERROR("Failed to vblank\n");
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 23/27] drm/vc4: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vc4.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vc4/vc4_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 6a1a9e1d72ce..f0b3e4cf5bce 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -880,7 +880,6 @@ int vc4_kms_load(struct drm_device *dev)
/* Set support for vblank irq fast disable, before drm_vblank_init() */
dev->vblank_disable_immediate = true;
- dev->irq_enabled = true;
ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize vblank\n");
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 21/27] drm/tegra: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in tegra. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/tegra/drm.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index f96c237b2242..8d27c21ddf48 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1188,13 +1188,6 @@ static int host1x_drm_probe(struct host1x_device *dev) goto device; } - /* -* We don't use the drm_irq_install() helpers provided by the DRM -* core, so we need to set this manually in order to allow the -* DRM_IOCTL_WAIT_VBLANK to operate correctly. -*/ - drm->irq_enabled = true; - /* syncpoints are used for full 32-bit hardware VBLANK counters */ drm->max_vblank_count = 0x; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 19/27] drm/stm: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in stm. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/stm/ltdc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 08b71248044d..e9c5a52f041a 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -1339,9 +1339,6 @@ int ltdc_load(struct drm_device *ddev) goto err; } - /* Allow usage of vblank without having to call drm_irq_install */ - ddev->irq_enabled = 1; - clk_disable_unprepare(ldev->pixel_clk); pinctrl_pm_select_sleep_state(ddev->dev); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 20/27] drm/sun4i: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sun4i. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index af335f58bdfc..570f3af25e86 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -97,8 +97,6 @@ static int sun4i_drv_bind(struct device *dev) if (ret) goto cleanup_mode_config; - drm->irq_enabled = true; - /* Remove early framebuffers (ie. simplefb) */ ret = drm_aperture_remove_framebuffers(false, "sun4i-drm-fb"); if (ret) -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 17/27] drm/rockchip: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rockchip. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index b730b8d5d949..c8e60fd9ff24 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -162,12 +162,6 @@ static int rockchip_drm_bind(struct device *dev) drm_mode_config_reset(drm_dev); - /* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -*/ - drm_dev->irq_enabled = true; - ret = rockchip_drm_fbdev_init(drm_dev); if (ret) goto err_unbind_all; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 18/27] drm/sti: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sti. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sti/sti_compositor.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index 319962a2c17b..9caaf3ccfabe 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -145,8 +145,6 @@ static int sti_compositor_bind(struct device *dev, } drm_vblank_init(drm_dev, crtc_id); - /* Allow usage of vblank without having to call drm_irq_install */ - drm_dev->irq_enabled = 1; return 0; } -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 16/27] drm/rcar-du: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rcar-du. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index bfbff90588cb..e289a66594a7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -593,8 +593,6 @@ static int rcar_du_probe(struct platform_device *pdev) goto error; } - rcdu->ddev.irq_enabled = 1; - /* * Register the DRM device with the core and the connectors with * sysfs. -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 15/27] drm/omapdrm: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct omap_drm_device.irq_enabled. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++
drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
b/drivers/gpu/drm/omapdrm/omap_drv.h
index d6f136984da9..591d4c273f02 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -48,6 +48,8 @@ struct omap_drm_private {
struct dss_device *dss;
struct dispc_device *dispc;
+ bool irq_enabled;
+
unsigned int num_pipes;
struct omap_drm_pipeline pipes[8];
struct omap_drm_pipeline *channels[8];
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
b/drivers/gpu/drm/omapdrm/omap_irq.c
index 15148d4b35b5..bb6e3fc18204 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -291,7 +291,7 @@ int omap_drm_irq_install(struct drm_device *dev)
if (ret < 0)
return ret;
- dev->irq_enabled = true;
+ priv->irq_enabled = true;
return 0;
}
@@ -300,10 +300,10 @@ void omap_drm_irq_uninstall(struct drm_device *dev)
{
struct omap_drm_private *priv = dev->dev_private;
- if (!dev->irq_enabled)
+ if (!priv->irq_enabled)
return;
- dev->irq_enabled = false;
+ priv->irq_enabled = false;
dispc_free_irq(priv->dispc, dev);
}
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 14/27] drm/nouveau: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in nouveau. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index a616cf4573b8..1cb14e99a60c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -553,8 +553,6 @@ nouveau_drm_device_init(struct drm_device *dev) if (ret) goto fail_master; - dev->irq_enabled = true; - nvxx_client(>client.base)->debug = nvkm_dbgopt(nouveau_debug, "DRM"); @@ -795,7 +793,6 @@ nouveau_drm_device_remove(struct drm_device *dev) drm_dev_unregister(dev); - dev->irq_enabled = false; client = nvxx_client(>client.base); device = nvkm_device_find(client->device); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 13/27] drm/mediatek: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in mediatek. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b46bdb8985da..9b60bec33d3b 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -270,12 +270,6 @@ static int mtk_drm_kms_init(struct drm_device *drm) goto err_component_unbind; } - /* -* We don't use the drm_irq_install() helpers provided by the DRM -* core, so we need to set this manually in order to allow the -* DRM_IOCTL_WAIT_VBLANK to operate correctly. -*/ - drm->irq_enabled = true; ret = drm_vblank_init(drm, MAX_CRTC); if (ret < 0) goto err_component_unbind; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 12/27] drm/imx/dcss: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in dcss. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Laurentiu Palcu Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c index 37ae68a7fba5..917834b1c80e 100644 --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c @@ -133,8 +133,6 @@ struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss) if (ret) goto cleanup_mode_config; - drm->irq_enabled = true; - ret = dcss_kms_bridge_connector_init(kms); if (ret) goto cleanup_mode_config; @@ -178,7 +176,6 @@ void dcss_kms_detach(struct dcss_kms_dev *kms) drm_kms_helper_poll_fini(drm); drm_atomic_helper_shutdown(drm); drm_crtc_vblank_off(>crtc.base); - drm->irq_enabled = false; drm_mode_config_cleanup(drm); dcss_crtc_deinit(>crtc, drm); drm->dev_private = NULL; -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 11/27] drm/imx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in imx. v3: * move dcss changes into separate patch (Laurentiu) Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/imx-drm-core.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 76819a8ac37f..9558e9e1b431 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -207,17 +207,6 @@ static int imx_drm_bind(struct device *dev) if (IS_ERR(drm)) return PTR_ERR(drm); - /* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -* -* P.S. note that we wouldn't use drm irq handler but -* just specific driver own one instead because -* drm framework supports only one irq handler and -* drivers can well take care of their interrupts -*/ - drm->irq_enabled = true; - /* * set max width and height as default value(4096x4096). * this value would be used to check framebuffer size limitation -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 10/27] drm/kirin: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in kirin.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index e590e19db657..98ae9a48f3fe 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -185,8 +185,6 @@ static int kirin_drm_kms_init(struct drm_device *dev,
DRM_ERROR("failed to initialize vblank.\n");
goto err_unbind_all;
}
- /* with irq_enabled = true, we can use the vblank feature. */
- dev->irq_enabled = true;
/* reset all the states of crtc/plane/encoder/connector */
drm_mode_config_reset(dev);
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 07/27] drm/komeda: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in komeda. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Liviu Dudau Acked-by: Daniel Vetter --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index ff45f23f3d56..52a6db5707a3 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -301,8 +301,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) if (err) goto free_component_binding; - drm->irq_enabled = true; - drm_kms_helper_poll_init(drm); err = drm_dev_register(drm, 0); @@ -313,7 +311,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) free_interrupts: drm_kms_helper_poll_fini(drm); - drm->irq_enabled = false; free_component_binding: component_unbind_all(mdev->dev, drm); cleanup_mode_config: @@ -331,7 +328,6 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) drm_dev_unregister(drm); drm_kms_helper_poll_fini(drm); drm_atomic_helper_shutdown(drm); - drm->irq_enabled = false; component_unbind_all(mdev->dev, drm); drm_mode_config_cleanup(drm); komeda_kms_cleanup_private_objs(kms); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 08/27] drm/malidp: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in malidp.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Liviu Dudau
Acked-by: Daniel Vetter
---
drivers/gpu/drm/arm/malidp_drv.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index de59f3302516..78d15b04b105 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -847,8 +847,6 @@ static int malidp_bind(struct device *dev)
if (ret < 0)
goto irq_init_fail;
- drm->irq_enabled = true;
-
ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
if (ret < 0) {
DRM_ERROR("failed to initialise vblank\n");
@@ -874,7 +872,6 @@ static int malidp_bind(struct device *dev)
vblank_fail:
malidp_se_irq_fini(hwdev);
malidp_de_irq_fini(hwdev);
- drm->irq_enabled = false;
irq_init_fail:
drm_atomic_helper_shutdown(drm);
component_unbind_all(dev, drm);
@@ -909,7 +906,6 @@ static void malidp_unbind(struct device *dev)
drm_atomic_helper_shutdown(drm);
malidp_se_irq_fini(hwdev);
malidp_de_irq_fini(hwdev);
- drm->irq_enabled = false;
component_unbind_all(dev, drm);
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 09/27] drm/exynos: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in exynos. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e60257f1f24b..d8f1cf4d6b69 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -300,16 +300,6 @@ static int exynos_drm_bind(struct device *dev) drm_mode_config_reset(drm); - /* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -* -* P.S. note that we wouldn't use drm irq handler but -* just specific driver own one instead because -* drm framework supports only one irq handler. -*/ - drm->irq_enabled = true; - /* init kms poll for handling hpd */ drm_kms_helper_poll_init(drm); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 05/27] drm/armada: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in armada. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/armada/armada_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index dab0a1f0983b..4a64f1b9ec4d 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -130,8 +130,6 @@ static int armada_drm_bind(struct device *dev) if (ret) goto err_comp; - priv->drm.irq_enabled = true; - drm_mode_config_reset(>drm); ret = armada_fbdev_init(>drm); -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 06/27] drm/i915: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct drm_i915_private.irq_enabled. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_irq.c | 8
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01e11fe38642..48c1835bd54b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,6 +1134,8 @@ struct drm_i915_private {
/* For i915gm/i945gm vblank irq workaround */
u8 vblank_enabled;
+ bool irq_enabled;
+
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a11bdb667241..987211f21761 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4488,14 +4488,14 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
*/
dev_priv->runtime_pm.irqs_enabled = true;
- dev_priv->drm.irq_enabled = true;
+ dev_priv->irq_enabled = true;
intel_irq_reset(dev_priv);
ret = request_irq(irq, intel_irq_handler(dev_priv),
IRQF_SHARED, DRIVER_NAME, dev_priv);
if (ret < 0) {
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
return ret;
}
@@ -4521,10 +4521,10 @@ void intel_irq_uninstall(struct drm_i915_private
*dev_priv)
* intel_modeset_driver_remove() calling us out of sequence.
* Would be nice if it didn't do that...
*/
- if (!dev_priv->drm.irq_enabled)
+ if (!dev_priv->irq_enabled)
return;
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
intel_irq_reset(dev_priv);
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for
vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state
for non-legacy drivers. For legacy drivers with userspace modesetting,
the original test remains in drm_wait_vblank_ioctl().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _device.dev and set as the device paramter in drm_dev_alloc().
*
- * These IRQ helpers are strictly optional. Drivers which roll their own only
- * need to set _device.irq_enabled to signal the DRM core that vblank
- * interrupts are working. Since these helpers don't automatically clean up the
- * requested interrupt like e.g. devm_request_irq() they're not really
+ * These IRQ helpers are strictly optional. Since these helpers don't
automatically
+ * clean up the requested interrupt like e.g. devm_request_irq() they're not
really
* recommended.
*/
@@ -91,9 +89,7 @@
* and after the installation.
*
* This is the simplified helper interface provided for drivers with no special
- * needs. Drivers which need to install interrupt handlers for multiple
- * interrupts must instead set _device.irq_enabled to signal the DRM core
- * that vblank interrupts are available.
+ * needs.
*
* @irq must match the interrupt number that would be passed to request_irq(),
* if called directly instead of using this helper function.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
- if (!dev->irq_enabled)
- return -EOPNOTSUPP;
+#if defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- if (!dev->irq_enabled)
+ if (!drm_dev_has_vblank(dev))
return -EOPNOTSUPP;
crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
@@ -2082,7 +2090,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EOPNOTSUPP;
- if (!dev->irq_enabled)
+ if (!drm_dev_has_vblank(dev))
return -EOPNOTSUPP;
crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 02/27] drm/hibmc: Call drm_irq_uninstall() unconditionally
Remove the check around drm_irq_uninstall(). The same test is
done by the function internally. The tested state in irq_enabled
is considered obsolete and should not be used by KMS drivers.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Tian Tao
Acked-by: Daniel Vetter
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -253,8 +253,7 @@ static int hibmc_unload(struct drm_device *dev)
{
drm_atomic_helper_shutdown(dev);
- if (dev->irq_enabled)
- drm_irq_uninstall(dev);
+ drm_irq_uninstall(dev);
pci_disable_msi(to_pci_dev(dev->dev));
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 03/27] drm/radeon: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct radeon_device.irq.installed. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/radeon/radeon_fence.c | 2 +-
drivers/gpu/drm/radeon/radeon_irq_kms.c | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index 0d8ef2368adf..7ec581363e23 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -288,7 +288,7 @@ static void radeon_fence_check_lockup(struct work_struct
*work)
return;
}
- if (fence_drv->delayed_irq && rdev->ddev->irq_enabled) {
+ if (fence_drv->delayed_irq && rdev->irq.installed) {
unsigned long irqflags;
fence_drv->delayed_irq = false;
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 84d0b1a3355f..a36ce826d0c0 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -357,7 +357,7 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev,
int ring)
{
unsigned long irqflags;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
if (atomic_inc_return(>irq.ring_int[ring]) == 1) {
@@ -396,7 +396,7 @@ void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev,
int ring)
{
unsigned long irqflags;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
if (atomic_dec_and_test(>irq.ring_int[ring])) {
@@ -422,7 +422,7 @@ void radeon_irq_kms_pflip_irq_get(struct radeon_device
*rdev, int crtc)
if (crtc < 0 || crtc >= rdev->num_crtc)
return;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
if (atomic_inc_return(>irq.pflip[crtc]) == 1) {
@@ -448,7 +448,7 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device
*rdev, int crtc)
if (crtc < 0 || crtc >= rdev->num_crtc)
return;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
if (atomic_dec_and_test(>irq.pflip[crtc])) {
@@ -470,7 +470,7 @@ void radeon_irq_kms_enable_afmt(struct radeon_device *rdev,
int block)
{
unsigned long irqflags;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
spin_lock_irqsave(>irq.lock, irqflags);
@@ -492,7 +492,7 @@ void radeon_irq_kms_disable_afmt(struct radeon_device
*rdev, int block)
{
unsigned long irqflags;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
spin_lock_irqsave(>irq.lock, irqflags);
@@ -514,7 +514,7 @@ void radeon_irq_kms_enable_hpd(struct radeon_device *rdev,
unsigned hpd_mask)
unsigned long irqflags;
int i;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
spin_lock_irqsave(>irq.lock, irqflags);
@@ -537,7 +537,7 @@ void radeon_irq_kms_disable_hpd(struct radeon_device *rdev,
unsigned hpd_mask)
unsigned long irqflags;
int i;
- if (!rdev->ddev->irq_enabled)
+ if (!rdev->irq.installed)
return;
spin_lock_irqsave(>irq.lock, irqflags);
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 01/27] drm/amdgpu: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct amdgpu_device.irq.installed. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 32ce0e679dc7..7dad44e73cf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -599,7 +599,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct
amdgpu_device *adev)
int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
unsigned type)
{
- if (!adev_to_drm(adev)->irq_enabled)
+ if (!adev->irq.installed)
return -ENOENT;
if (type >= src->num_types)
@@ -629,7 +629,7 @@ int amdgpu_irq_get(struct amdgpu_device *adev, struct
amdgpu_irq_src *src,
int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
unsigned type)
{
- if (!adev_to_drm(adev)->irq_enabled)
+ if (!adev->irq.installed)
return -ENOENT;
if (type >= src->num_types)
@@ -660,7 +660,7 @@ int amdgpu_irq_put(struct amdgpu_device *adev, struct
amdgpu_irq_src *src,
bool amdgpu_irq_enabled(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
unsigned type)
{
- if (!adev_to_drm(adev)->irq_enabled)
+ if (!adev->irq.installed)
return false;
if (type >= src->num_types)
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 00/27] Deprecate struct drm_device.irq_enabled
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core. KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs. The first 3 patches replace uses of irq_enabled that are not required. Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs (for KMS drivers). The rest of the patchset removes irq_enabled from all non-legacy drivers. The only exceptions are i915 and omapdrm, which have an internal dpendency on the field's value. For these drivers, the state gets duplicated internally. With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy. v3: * update armada, i915, rcar-du and vkms as well (Laurent) * optimize drm_wait_vblank_ioctl() for KMS (Liviu) * move imx/dcss changes into their own patch (Laurentiu) * doc cleanups v2: * keep the original test for legacy drivers in drm_wait_vblank_ioctl() (Daniel) Thomas Zimmermann (27): drm/amdgpu: Track IRQ state in local device state drm/hibmc: Call drm_irq_uninstall() unconditionally drm/radeon: Track IRQ state in local device state drm: Don't test for IRQ support in VBLANK ioctls drm/armada: Don't set struct drm_device.irq_enabled drm/i915: Track IRQ state in local device state drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/imx/dcss: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rcar-du: Don't set struct drm_device.irq_enabled drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabled drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vkms: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enabled drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 drivers/gpu/drm/arm/malidp_drv.c| 4 drivers/gpu/drm/armada/armada_drv.c | 2 -- drivers/gpu/drm/drm_irq.c | 13 - drivers/gpu/drm/drm_vblank.c| 16 drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 -- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 8 drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 -- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 -- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 -- drivers/gpu/drm/sti/sti_compositor.c| 2 -- drivers/gpu/drm/stm/ltdc.c | 3 --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- drivers/gpu/drm/tegra/drm.c | 7 --- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - drivers/gpu/drm/vkms/vkms_drv.c | 2 -- drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- drivers/gpu/drm/zte/zx_drm_drv.c| 6 -- 31 files changed, 40 insertions(+), 123 deletions(-) base-commit: 8c1323b422f8473421682ba783b5949ddd89a3f4 prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.32.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:39:48PM +0300, Oded Gabbay wrote: > On Wed, Jun 23, 2021 at 10:34 PM Jason Gunthorpe wrote: > > > > On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > > > On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > > > > > > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > > > > > > > Can you please explain why it is so important to (allow) access them > > > > > through the CPU ? > > > > > > > > It is not so much important, as it reflects significant design choices > > > > that are already tightly baked into alot of our stacks. > > > > > > > > A SGL is CPU accessible by design - that is baked into this thing and > > > > places all over the place assume it. Even in RDMA we have > > > > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > > > > to see) > > > > > > > > So, the thing at the top of the stack - in this case the gaudi driver > > > > - simply can't assume what the rest of the stack is going to do and > > > > omit the CPU side. It breaks everything. > > > > > > > > Logan's patch series is the most fully developed way out of this > > > > predicament so far. > > > > > > I understand the argument and I agree that for the generic case, the > > > top of the stack can't assume anything. > > > Having said that, in this case the SGL is encapsulated inside a dma-buf > > > object. > > > > > > Maybe its a stupid/over-simplified suggestion, but can't we add a > > > property to the dma-buf object, > > > that will be set by the exporter, which will "tell" the importer it > > > can't use any CPU fallback ? Only "real" p2p ? > > > > The block stack has been trying to do something like this. > > > > The flag doesn't solve the DMA API/IOMMU problems though. > hmm, I thought using dma_map_resource will solve the IOMMU issues, > no ? dma_map_resource() will configure the IOMMU but it is not the correct API to use when building a SG list for DMA, that would be dma_map_sg or sgtable. So it works, but it is an API abuse to build things this way. > If I use dma_map_resource to set the addresses inside the SGL before I > export the dma-buf, and guarantee no one will use the SGL in the > dma-buf for any other purpose than device p2p, what else is needed ? You still have to check the p2p stuff to ensure that p2p is even possible And this approach is misusing all the APIs and has been NAK'd by Christoph, so up to Greg if he wants to take it or insist you work with Logan to get the proper generlized solution finished. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/dc: Really fix DCN3.1 Makefile for PPC64
Also copy over the part that makes old gcc handling cross-platform.
Fixes: df7a1658f257 ("drm/amdgpu/dc: fix DCN3.1 Makefile for PPC64")
Fixes: 926d6972efb6 ("drm/amd/display: Add DCN3.1 blocks to the DC Makefile")
Signed-off-by: Michal Suchanek
---
The fact that the old gcc handling triggers on gcc 10 and 11 is another
story I don't want to delve into.
---
drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/Makefile
b/drivers/gpu/drm/amd/display/dc/dcn31/Makefile
index 5dcdc5a858fe..4bab97acb155 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/Makefile
@@ -28,6 +28,7 @@ endif
CFLAGS_$(AMDDALPATH)/dc/dcn31/dcn31_resource.o += -mhard-float
endif
+ifdef CONFIG_X86
ifdef IS_OLD_GCC
# Stack alignment mismatch, proceed with caution.
# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3
@@ -36,6 +37,7 @@ CFLAGS_$(AMDDALPATH)/dc/dcn31/dcn31_resource.o +=
-mpreferred-stack-boundary=4
else
CFLAGS_$(AMDDALPATH)/dc/dcn31/dcn31_resource.o += -msse2
endif
+endif
AMD_DAL_DCN31 = $(addprefix $(AMDDALPATH)/dc/dcn31/,$(DCN31))
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:39:48PM +0300, Oded Gabbay wrote: > hmm, I thought using dma_map_resource will solve the IOMMU issues, no ? > We talked about it yesterday, and you said that it will "work" > (although I noticed a tone of reluctance when you said that). > > If I use dma_map_resource to set the addresses inside the SGL before I > export the dma-buf, and guarantee no one will use the SGL in the > dma-buf for any other purpose than device p2p, what else is needed ? dma_map_resource works in the sense of that helps with mapping an arbitrary phys_addr_t for DMA. It does not take various pitfalls of PCI P2P into account, such as the offset between the CPU physical address and the PCIe bus address, or the whole support of mapping between two devices behding a switch and not going through the limited root port support. Comparing dma_direct_map_resource/iommu_dma_map_resource with with pci_p2pdma_map_sg_attrs/__pci_p2pdma_map_sg should make that very clear. So if you want a non-page based mapping you need a "resource"-level version of pci_p2pdma_map_sg_attrs. Which totall doable, and in fact mostly trivial. But no one has even looked into providing one and just keeps arguing. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3 0/8] Support DEVICE_GENERIC memory in migrate_vma_*
On Wed, Jun 23, 2021 at 05:49:55PM -0400, Felix Kuehling wrote: > For the reference counting changes we could use the dax driver with hmem > and use efi_fake_mem on the kernel command line to create some > DEVICE_GENERIC pages. I'm open to suggestions for good user mode tests to > exercise dax functionality on this type of memory. > > For the migration helper changes we could modify or parametrize > lib/hmm_test.c to create DEVICE_GENERIC pages instead of DEVICE_PRIVATE. > Then run tools/testing/selftests/vm/hmm-tests.c. We'll also need a real in-tree user of the enhanced DEVICE_GENERIC memory. So while the refcounting cleanups early in the series are something I'd really like to see upstream as soon as everything is sorted out, the actual bits that can't only be used by your updated driver should wait for that. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > I understand the argument and I agree that for the generic case, the > top of the stack can't assume anything. > Having said that, in this case the SGL is encapsulated inside a dma-buf > object. But the scatterlist is defined to have a valid page. If in dma-bufs you can't do that dmabufs are completely broken. Apparently the gpu folks can somehow live with that and deal with the pitfals, but for dma-buf users outside of their little fiefdom were they arbitrarily break rules it simply is not acceptable. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
答复: [PATCH v2 02/22] drm/hibmc: Call drm_irq_uninstall() unconditionally
-邮件原件-
发件人: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
发送时间: 2021年6月22日 22:10
收件人: dan...@ffwll.ch; airl...@linux.ie; alexander.deuc...@amd.com;
christian.koe...@amd.com; xinhui@amd.com; james.qian.w...@arm.com;
liviu.du...@arm.com; mihail.atanas...@arm.com; brian.star...@arm.com;
maarten.lankho...@linux.intel.com; mrip...@kernel.org; inki@samsung.com;
jy0922.s...@samsung.com; sw0312@samsung.com; kyungmin.p...@samsung.com;
krzysztof.kozlow...@canonical.com; xinliang@linaro.org; tiantao (H)
; john.stu...@linaro.org; kongxinwei (A)
; Chenfeng (puck) ;
laurentiu.pa...@oss.nxp.com; l.st...@pengutronix.de; p.za...@pengutronix.de;
shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
feste...@gmail.com; linux-...@nxp.com; chunkuang...@kernel.org;
matthias@gmail.com; bske...@redhat.com; to...@kernel.org;
h...@rock-chips.com; he...@sntech.de; benjamin.gaign...@linaro.org;
yannick.fer...@foss.st.com; philippe.co...@foss.st.com;
mcoquelin.st...@gmail.com; alexandre.tor...@foss.st.com; w...@csie.org;
jernej.skra...@gmail.com; thierry.red...@gmail.com; jonath...@nvidia.com;
jyri.sa...@iki.fi; e...@anholt.net; linux-graphics-maintai...@vmware.com;
za...@vmware.com; hyun.k...@xilinx.com; laurent.pinch...@ideasonboard.com;
michal.si...@xilinx.com
抄送: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
linux-arm-ker...@lists.infradead.org; linux-samsung-...@vger.kernel.org;
linux-media...@lists.infradead.org; nouv...@lists.freedesktop.org;
linux-rockc...@lists.infradead.org; linux-st...@st-md-mailman.stormreply.com;
linux-su...@lists.linux.dev; linux-te...@vger.kernel.org; Thomas Zimmermann
主题: [PATCH v2 02/22] drm/hibmc: Call drm_irq_uninstall() unconditionally
Remove the check around drm_irq_uninstall(). The same test is done by the
function internally. The tested state in irq_enabled is considered obsolete and
should not be used by KMS drivers.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Tian Tao
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -253,8 +253,7 @@ static int hibmc_unload(struct drm_device *dev) {
drm_atomic_helper_shutdown(dev);
- if (dev->irq_enabled)
- drm_irq_uninstall(dev);
+ drm_irq_uninstall(dev);
pci_disable_msi(to_pci_dev(dev->dev));
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3] drm/radeon: Fix NULL dereference when updating memory stats
radeon_ttm_bo_destroy() is attempting to access the resource object to
update memory counters. However, the resource object is already freed when
ttm calls this function via the destroy callback. This causes an oops when
a bo is freed:
BUG: kernel NULL pointer dereference, address: 0010
RIP: 0010:radeon_ttm_bo_destroy+0x2c/0x100 [radeon]
Call Trace:
radeon_bo_unref+0x1a/0x30 [radeon]
radeon_gem_object_free+0x33/0x50 [radeon]
drm_gem_object_release_handle+0x69/0x70 [drm]
drm_gem_handle_delete+0x62/0xa0 [drm]
? drm_mode_destroy_dumb+0x40/0x40 [drm]
drm_ioctl_kernel+0xb2/0xf0 [drm]
drm_ioctl+0x30a/0x3c0 [drm]
? drm_mode_destroy_dumb+0x40/0x40 [drm]
radeon_drm_ioctl+0x49/0x80 [radeon]
__x64_sys_ioctl+0x8e/0xd0
Avoid the issue by updating the counters in the delete_mem_notify callback
instead. Also, fix memory statistic updating in radeon_bo_move() to
identify the source type correctly. The source type needs to be saved
before the move, because the moved from object may be altered by the move.
Fixes: bfa3357ef9ab ("drm/ttm: allocate resource object instead of embedding it
v2")
Signed-off-by: Mikel Rychliski
---
drivers/gpu/drm/radeon/radeon_object.c | 29 -
drivers/gpu/drm/radeon/radeon_object.h | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c| 13 ++---
3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c
b/drivers/gpu/drm/radeon/radeon_object.c
index bfaaa3c969a3..56ede9d63b12 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -49,23 +49,23 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo
*bo);
* function are calling it.
*/
-static void radeon_update_memory_usage(struct radeon_bo *bo,
- unsigned mem_type, int sign)
+static void radeon_update_memory_usage(struct ttm_buffer_object *bo,
+ unsigned int mem_type, int sign)
{
- struct radeon_device *rdev = bo->rdev;
+ struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
switch (mem_type) {
case TTM_PL_TT:
if (sign > 0)
- atomic64_add(bo->tbo.base.size, >gtt_usage);
+ atomic64_add(bo->base.size, >gtt_usage);
else
- atomic64_sub(bo->tbo.base.size, >gtt_usage);
+ atomic64_sub(bo->base.size, >gtt_usage);
break;
case TTM_PL_VRAM:
if (sign > 0)
- atomic64_add(bo->tbo.base.size, >vram_usage);
+ atomic64_add(bo->base.size, >vram_usage);
else
- atomic64_sub(bo->tbo.base.size, >vram_usage);
+ atomic64_sub(bo->base.size, >vram_usage);
break;
}
}
@@ -76,8 +76,6 @@ static void radeon_ttm_bo_destroy(struct ttm_buffer_object
*tbo)
bo = container_of(tbo, struct radeon_bo, tbo);
- radeon_update_memory_usage(bo, bo->tbo.resource->mem_type, -1);
-
mutex_lock(>rdev->gem.mutex);
list_del_init(>list);
mutex_unlock(>rdev->gem.mutex);
@@ -727,24 +725,21 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool
has_moved,
}
void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- bool evict,
+ unsigned int old_type,
struct ttm_resource *new_mem)
{
struct radeon_bo *rbo;
+ radeon_update_memory_usage(bo, old_type, -1);
+ if (new_mem)
+ radeon_update_memory_usage(bo, new_mem->mem_type, 1);
+
if (!radeon_ttm_bo_is_radeon_bo(bo))
return;
rbo = container_of(bo, struct radeon_bo, tbo);
radeon_bo_check_tiling(rbo, 0, 1);
radeon_vm_bo_invalidate(rbo->rdev, rbo);
-
- /* update statistics */
- if (!new_mem)
- return;
-
- radeon_update_memory_usage(rbo, bo->resource->mem_type, -1);
- radeon_update_memory_usage(rbo, new_mem->mem_type, 1);
}
vm_fault_t radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_object.h
b/drivers/gpu/drm/radeon/radeon_object.h
index 1739c6a142cd..1afc7992ef91 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -161,7 +161,7 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
bool force_drop);
extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- bool evict,
+ unsigned int old_type,
struct ttm_resource *new_mem);
extern vm_fault_t
[PATCH] drm/amdgpu:use kvcalloc instead of kvmalloc_array
kvmalloc_array + __GFP_ZERO is the same with kvcalloc. Signed-off-by: huqiqiao --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 9acee4a5b2ba..50edc73525b0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -908,9 +908,8 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev, unsigned num_entries; num_entries = amdgpu_vm_num_entries(adev, cursor->level); - entry->entries = kvmalloc_array(num_entries, - sizeof(*entry->entries), - GFP_KERNEL | __GFP_ZERO); + entry->entries = kvcalloc(num_entries, + sizeof(*entry->entries), GFP_KERNEL); if (!entry->entries) return -ENOMEM; } -- 2.11.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:00:29PM +0300, Oded Gabbay wrote: > On Wed, Jun 23, 2021 at 9:50 PM Jason Gunthorpe wrote: > > > > On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > > > > > Can you please explain why it is so important to (allow) access them > > > through the CPU ? > > > > It is not so much important, as it reflects significant design choices > > that are already tightly baked into alot of our stacks. > > > > A SGL is CPU accessible by design - that is baked into this thing and > > places all over the place assume it. Even in RDMA we have > > RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page > > to see) > > > > So, the thing at the top of the stack - in this case the gaudi driver > > - simply can't assume what the rest of the stack is going to do and > > omit the CPU side. It breaks everything. > > > > Logan's patch series is the most fully developed way out of this > > predicament so far. > > I understand the argument and I agree that for the generic case, the > top of the stack can't assume anything. > Having said that, in this case the SGL is encapsulated inside a dma-buf > object. > > Maybe its a stupid/over-simplified suggestion, but can't we add a > property to the dma-buf object, > that will be set by the exporter, which will "tell" the importer it > can't use any CPU fallback ? Only "real" p2p ? The block stack has been trying to do something like this. The flag doesn't solve the DMA API/IOMMU problems though. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 10:57:35AM +0200, Christian König wrote: > > > No it isn't. It makes devices depend on allocating struct pages for their > > > BARs which is not necessary nor desired. > > Which dramatically reduces the cost of establishing DMA mappings, a > > loop of dma_map_resource() is very expensive. > > Yeah, but that is perfectly ok. Our BAR allocations are either in chunks of > at least 2MiB or only a single 4KiB page. And very small apparently > > > Allocating a struct pages has their use case, for example for exposing > > > VRAM > > > as memory for HMM. But that is something very specific and should not > > > limit > > > PCIe P2P DMA in general. > > Sure, but that is an ideal we are far from obtaining, and nobody wants > > to work on it prefering to do hacky hacky like this. > > > > If you believe in this then remove the scatter list from dmabuf, add a > > new set of dma_map* APIs to work on physical addresses and all the > > other stuff needed. > > Yeah, that's what I totally agree on. And I actually hoped that the new P2P > work for PCIe would go into that direction, but that didn't materialized. It is a lot of work and the only gain is to save a bit of memory for struct pages. Not a very big pay off. > But allocating struct pages for PCIe BARs which are essentially registers > and not memory is much more hacky than the dma_resource_map() approach. It doesn't really matter. The pages are in a special zone and are only being used as handles for the BAR memory. > By using PCIe P2P we want to avoid the round trip to the CPU when one device > has filled the ring buffer and another device must be woken up to process > it. Sure, we all have these scenarios, what is inside the memory doesn't realy matter. The mechanism is generic and the struct pages don't care much if they point at something memory-like or at something register-like. They are already in big trouble because you can't portably use CPU instructions to access them anyhow. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Linaro-mm-sig] [PATCH v3 1/2] habanalabs: define uAPI to export FD for DMA-BUF
On Wed, Jun 23, 2021 at 09:43:04PM +0300, Oded Gabbay wrote: > Can you please explain why it is so important to (allow) access them > through the CPU ? It is not so much important, as it reflects significant design choices that are already tightly baked into alot of our stacks. A SGL is CPU accessible by design - that is baked into this thing and places all over the place assume it. Even in RDMA we have RXE/SWI/HFI1/qib that might want to use the CPU side (grep for sg_page to see) So, the thing at the top of the stack - in this case the gaudi driver - simply can't assume what the rest of the stack is going to do and omit the CPU side. It breaks everything. Logan's patch series is the most fully developed way out of this predicament so far. > The whole purpose is that the other device accesses my device, > bypassing the CPU. Sure, but you don't know that will happen, or if it is even possible in any given system configuration. The purpose is to allow for that optimization when possible, not exclude CPU based approaches. Jason ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/radeon: Fix NULL dereference when updating memory stats
On Wednesday, June 23, 2021 2:55:04 AM EDT Christian König wrote: > Please rather keep the new resource as parameter here and update before > adjusting bo->resource. > > This way you also don't need to export radeon_update_memory_usage(). I wasn't sure exactly what you intended with the request to "update before adjusting bo->resource". Assuming the statistics update is done as part of radeon_bo_move_notify(), I believe that function cannot be called any earlier in radeon_bo_move(). If it were, the source object would be invalidated before it moved. So I assume you're suggesting updating the memory usage earlier in bo_move_notify (before the early return for ghost objects). Thanks, Mikel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
