Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces

[<Dan.Sneddon@microchip.com>]

On 7/28/21 7:00 AM, Sam Ravnborg wrote:
> [You don't often get email from sam@ravnborg.org. Learn why this is important at http://aka.ms/LearnAboutSenderIdentification.]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Dan,
> 
> I hope you can fine to test this patch from Thomas.
> If this works then we can forget about my attempt to do the same.

I'll test this as soon as I can and let you know.

Thanks,
Dan
> 
> Hi Thomas,
> 
> IRQ_NOTCONNECTED check seems redundant, as mentioned in another patch
> already.
> 
> With that considered:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> We shall wait for testing from Dan before you apply it as I had made a
> similar patch that failed to work. I assume my patch was buggy but I
> prefer to be sure.
> 
>          Sam
> 
> On Tue, Jul 27, 2021 at 08:27:10PM +0200, Thomas Zimmermann wrote:
>> 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.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 85 ++++++++++++--------
>>   1 file changed, 52 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index f09b6dd8754c..cfa8c2c9c8aa 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -22,7 +22,6 @@
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_gem_cma_helper.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>> -#include <drm/drm_irq.h>
>>   #include <drm/drm_probe_helper.h>
>>   #include <drm/drm_vblank.h>
>>
>> @@ -557,6 +556,56 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data)
>>        return IRQ_HANDLED;
>>   }
>>
>> +static void atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
>> +{
>> +     struct atmel_hlcdc_dc *dc = dev->dev_private;
>> +     unsigned int cfg = 0;
>> +     int i;
>> +
>> +     /* Enable interrupts on activated layers */
>> +     for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
>> +             if (dc->layers[i])
>> +                     cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
>> +     }
>> +
>> +     regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
>> +}
>> +
>> +static void atmel_hlcdc_dc_irq_disable(struct drm_device *dev)
>> +{
>> +     struct atmel_hlcdc_dc *dc = dev->dev_private;
>> +     unsigned int isr;
>> +
>> +     regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0xffffffff);
>> +     regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, &isr);
>> +}
>> +
>> +static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int irq)
>> +{
>> +     int ret;
>> +
>> +     if (irq == IRQ_NOTCONNECTED)
>> +             return -ENOTCONN;
>> +
>> +     atmel_hlcdc_dc_irq_disable(dev);
>> +
>> +     ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0, dev->driver->name, dev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     atmel_hlcdc_dc_irq_postinstall(dev);
>> +
>> +     return 0;
>> +}
>> +
>> +static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
>> +{
>> +     struct atmel_hlcdc_dc *dc = dev->dev_private;
>> +
>> +     atmel_hlcdc_dc_irq_disable(dev);
>> +     free_irq(dc->hlcdc->irq, dev);
>> +}
>> +
>>   static const struct drm_mode_config_funcs mode_config_funcs = {
>>        .fb_create = drm_gem_fb_create,
>>        .atomic_check = drm_atomic_helper_check,
>> @@ -647,7 +696,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>>        drm_mode_config_reset(dev);
>>
>>        pm_runtime_get_sync(dev->dev);
>> -     ret = drm_irq_install(dev, dc->hlcdc->irq);
>> +     ret = atmel_hlcdc_dc_irq_install(dev, dc->hlcdc->irq);
>>        pm_runtime_put_sync(dev->dev);
>>        if (ret < 0) {
>>                dev_err(dev->dev, "failed to install IRQ handler\n");
>> @@ -676,7 +725,7 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
>>        drm_mode_config_cleanup(dev);
>>
>>        pm_runtime_get_sync(dev->dev);
>> -     drm_irq_uninstall(dev);
>> +     atmel_hlcdc_dc_irq_uninstall(dev);
>>        pm_runtime_put_sync(dev->dev);
>>
>>        dev->dev_private = NULL;
>> @@ -685,40 +734,10 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
>>        clk_disable_unprepare(dc->hlcdc->periph_clk);
>>   }
>>
>> -static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
>> -{
>> -     struct atmel_hlcdc_dc *dc = dev->dev_private;
>> -     unsigned int cfg = 0;
>> -     int i;
>> -
>> -     /* Enable interrupts on activated layers */
>> -     for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
>> -             if (dc->layers[i])
>> -                     cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
>> -     }
>> -
>> -     regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
>> -
>> -     return 0;
>> -}
>> -
>> -static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
>> -{
>> -     struct atmel_hlcdc_dc *dc = dev->dev_private;
>> -     unsigned int isr;
>> -
>> -     regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0xffffffff);
>> -     regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, &isr);
>> -}
>> -
>>   DEFINE_DRM_GEM_CMA_FOPS(fops);
>>
>>   static const struct drm_driver atmel_hlcdc_dc_driver = {
>>        .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> -     .irq_handler = atmel_hlcdc_dc_irq_handler,
>> -     .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>> -     .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>> -     .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>>        DRM_GEM_CMA_DRIVER_OPS,
>>        .fops = &fops,
>>        .name = "atmel-hlcdc",
>> --
>> 2.32.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel