Hi

Am 28.07.21 um 16:03 schrieb Alex Deucher:
> On Wed, Jul 28, 2021 at 6:27 AM Christian König
> <christian.koenig@amd.com> wrote:
>>
>> Am 27.07.21 um 20:27 schrieb Thomas Zimmermann:
>>> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
>>> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
>>> don't benefit from using it.
>>>
>>> DRM IRQ callbacks are now being called directly or inlined.
>>>
>>> The interrupt number returned by pci_msi_vector() is now stored
>>> in struct amdgpu_irq. Calls to pci_msi_vector() can fail and return
>>> a negative errno code. Abort initlaizaton in thi case. The DRM IRQ
>>> midlayer does not handle this correctly.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> Alex needs to take a look at this as well, but of hand the patch is
>> Acked-by: Christian König <christian.koenig@amd.com>.
> 
> Looks good to me as well:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks a lot. Do you have comments on the changes to radeon?

Best regards
Thomas

> 
>>
>> Thanks,
>> Christian.
>>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 21 ++++++++++++++-------
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  2 +-
>>>    3 files changed, 15 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 2bd13fc2541a..1e05b5aa94e7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -1775,7 +1775,6 @@ static const struct drm_driver amdgpu_kms_driver = {
>>>        .open = amdgpu_driver_open_kms,
>>>        .postclose = amdgpu_driver_postclose_kms,
>>>        .lastclose = amdgpu_driver_lastclose_kms,
>>> -     .irq_handler = amdgpu_irq_handler,
>>>        .ioctls = amdgpu_ioctls_kms,
>>>        .num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms),
>>>        .dumb_create = amdgpu_mode_dumb_create,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> index 0d01cfaca77e..a36cdc7323f4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> @@ -46,7 +46,6 @@
>>>    #include <linux/pci.h>
>>>
>>>    #include <drm/drm_crtc_helper.h>
>>> -#include <drm/drm_irq.h>
>>>    #include <drm/drm_vblank.h>
>>>    #include <drm/amdgpu_drm.h>
>>>    #include <drm/drm_drv.h>
>>> @@ -184,7 +183,7 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
>>>     * Returns:
>>>     * result of handling the IRQ, as defined by &irqreturn_t
>>>     */
>>> -irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>>> +static irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>>>    {
>>>        struct drm_device *dev = (struct drm_device *) arg;
>>>        struct amdgpu_device *adev = drm_to_adev(dev);
>>> @@ -307,6 +306,7 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
>>>    int amdgpu_irq_init(struct amdgpu_device *adev)
>>>    {
>>>        int r = 0;
>>> +     unsigned int irq;
>>>
>>>        spin_lock_init(&adev->irq.lock);
>>>
>>> @@ -349,15 +349,22 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>>>        INIT_WORK(&adev->irq.ih2_work, amdgpu_irq_handle_ih2);
>>>        INIT_WORK(&adev->irq.ih_soft_work, amdgpu_irq_handle_ih_soft);
>>>
>>> -     adev->irq.installed = true;
>>> -     /* Use vector 0 for MSI-X */
>>> -     r = drm_irq_install(adev_to_drm(adev), pci_irq_vector(adev->pdev, 0));
>>> +     /* Use vector 0 for MSI-X. */
>>> +     r = pci_irq_vector(adev->pdev, 0);
>>> +     if (r < 0)
>>> +             return r;
>>> +     irq = r;
>>> +
>>> +     /* PCI devices require shared interrupts. */
>>> +     r = request_irq(irq, amdgpu_irq_handler, IRQF_SHARED, adev_to_drm(adev)->driver->name,
>>> +                     adev_to_drm(adev));
>>>        if (r) {
>>> -             adev->irq.installed = false;
>>>                if (!amdgpu_device_has_dc_support(adev))
>>>                        flush_work(&adev->hotplug_work);
>>>                return r;
>>>        }
>>> +     adev->irq.installed = true;
>>> +     adev->irq.irq = irq;
>>>        adev_to_drm(adev)->max_vblank_count = 0x00ffffff;
>>>
>>>        DRM_DEBUG("amdgpu: irq initialized.\n");
>>> @@ -368,7 +375,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>>>    void amdgpu_irq_fini_hw(struct amdgpu_device *adev)
>>>    {
>>>        if (adev->irq.installed) {
>>> -             drm_irq_uninstall(&adev->ddev);
>>> +             free_irq(adev->irq.irq, adev_to_drm(adev));
>>>                adev->irq.installed = false;
>>>                if (adev->irq.msi_enabled)
>>>                        pci_free_irq_vectors(adev->pdev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
>>> index 78ad4784cc74..e9f2c11ea416 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
>>> @@ -80,6 +80,7 @@ struct amdgpu_irq_src_funcs {
>>>
>>>    struct amdgpu_irq {
>>>        bool                            installed;
>>> +     unsigned int                    irq;
>>>        spinlock_t                      lock;
>>>        /* interrupt sources */
>>>        struct amdgpu_irq_client        client[AMDGPU_IRQ_CLIENTID_MAX];
>>> @@ -100,7 +101,6 @@ struct amdgpu_irq {
>>>    };
>>>
>>>    void amdgpu_irq_disable_all(struct amdgpu_device *adev);
>>> -irqreturn_t amdgpu_irq_handler(int irq, void *arg);
>>>
>>>    int amdgpu_irq_init(struct amdgpu_device *adev);
>>>    void amdgpu_irq_fini_sw(struct amdgpu_device *adev);
>>

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