Re: Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
On 7/30/21 1:31 AM, Thomas Zimmermann wrote:
> Hi Dan and Sam
>
> Am 29.07.21 um 21:55 schrieb dan.sned...@microchip.com:
>> Hi Thomas and Sam,
>> On 7/29/21 12:48 PM, Sam Ravnborg wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>> know the content is safe
>>>
>>> Hi Thomas,
>>>
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently
replaced
by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK
ioctls").
>>
>> Ok, My fault for testing on the wrong branch. When I test this patch on
>> drm-misc-next it works great. Sorry for the confusion!
>>
>>>
>>> I was looking at drm-misc-fixes which did not have this commit :-(
>>> Just my silly excuse why I was convinced this was the issue.
>
> Don't worry.
>
> I'll add Sam's R-b and a Tested-by from Dan to the patch. Is that ok?
The tested-by works for me! Thanks!
>
> Best regards
> Thomas
>
>
>>>
>>> Sam
>>>
>>
>> Best regards,
>> Dan
>>
>
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Dan and Sam
Am 29.07.21 um 21:55 schrieb dan.sned...@microchip.com:
Hi Thomas and Sam,
On 7/29/21 12:48 PM, Sam Ravnborg wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe
Hi Thomas,
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently replaced
by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK ioctls").
Ok, My fault for testing on the wrong branch. When I test this patch on
drm-misc-next it works great. Sorry for the confusion!
I was looking at drm-misc-fixes which did not have this commit :-(
Just my silly excuse why I was convinced this was the issue.
Don't worry.
I'll add Sam's R-b and a Tested-by from Dan to the patch. Is that ok?
Best regards
Thomas
Sam
Best regards,
Dan
--
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: Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Thomas,
On 7/29/21 12:18 PM, Thomas Zimmermann wrote:
> Hi
>
> Am 28.07.21 um 22:11 schrieb Sam Ravnborg:
>> Hi Dan,
>>
I think I got it - we need to set irq_enabled to true.
The documentation says so:
"
* @irq_enabled:
*
* Indicates that interrupt handling is enabled,
specifically vblank
* handling. Drivers which don't use drm_irq_install()
need to set this
* to true manually.
"
Can you try to add the following line:
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev,
unsigned int irq)
+{
+ int ret;
+
+ if (irq == IRQ_NOTCONNECTED)
+ return -ENOTCONN;
+
dev->irq_enabled = true; <= THIS LINE
+ atmel_hlcdc_dc_irq_disable(dev);
+
+ ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
+ if (ret)
+ return ret;
I hope this fixes it.
>>>
>>> It does! With the irq_enabled line added everything is looking good.
>
> Are you sure, you're testing with the latest drm-misc-next or drm-tip?
> Because using irq_enabled is deprecated and the flag was recently
> replaced by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in
> VBLANK ioctls").
>
> Best regards
> Thomas
>
I was testing with 5.14-rc3. I can test with drm-tip or drm-misc-next.
There a preferred branch to test from?
Thanks and regards,
Dan
>>
>> Great, thanks for testing.
>>
>> Thomas - I assume you will do a re-spin and there is likely some fixes
>> for the applied IRQ conversions too.
>>
>> Note - irq_enabled must be cleared if request_irq fails. I did not
>> include this in the testing here.
>>
>> Sam
>>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Thomas and Sam,
On 7/29/21 12:48 PM, Sam Ravnborg wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi Thomas,
>
>>
>> Are you sure, you're testing with the latest drm-misc-next or drm-tip?
>> Because using irq_enabled is deprecated and the flag was recently replaced
>> by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK ioctls").
Ok, My fault for testing on the wrong branch. When I test this patch on
drm-misc-next it works great. Sorry for the confusion!
>
> I was looking at drm-misc-fixes which did not have this commit :-(
> Just my silly excuse why I was convinced this was the issue.
>
> Sam
>
Best regards,
Dan
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Thomas,
>
> Are you sure, you're testing with the latest drm-misc-next or drm-tip?
> Because using irq_enabled is deprecated and the flag was recently replaced
> by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in VBLANK ioctls").
I was looking at drm-misc-fixes which did not have this commit :-(
Just my silly excuse why I was convinced this was the issue.
Sam
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi
Am 29.07.21 um 21:24 schrieb dan.sned...@microchip.com:
Hi Thomas,
On 7/29/21 12:18 PM, Thomas Zimmermann wrote:
Hi
Am 28.07.21 um 22:11 schrieb Sam Ravnborg:
Hi Dan,
I think I got it - we need to set irq_enabled to true.
The documentation says so:
"
* @irq_enabled:
*
* Indicates that interrupt handling is enabled,
specifically vblank
* handling. Drivers which don't use drm_irq_install()
need to set this
* to true manually.
"
Can you try to add the following line:
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev,
unsigned int irq)
+{
+ int ret;
+
+ if (irq == IRQ_NOTCONNECTED)
+ return -ENOTCONN;
+
dev->irq_enabled = true; <= THIS LINE
+ atmel_hlcdc_dc_irq_disable(dev);
+
+ ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
+ if (ret)
+ return ret;
I hope this fixes it.
It does! With the irq_enabled line added everything is looking good.
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently
replaced by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in
VBLANK ioctls").
Best regards
Thomas
I was testing with 5.14-rc3. I can test with drm-tip or drm-misc-next.
There a preferred branch to test from?
I use drm-tip for development, but all the relevant patches go through
drm-misc-next. So either is fine.
Best regards
Thomas
Thanks and regards,
Dan
Great, thanks for testing.
Thomas - I assume you will do a re-spin and there is likely some fixes
for the applied IRQ conversions too.
Note - irq_enabled must be cleared if request_irq fails. I did not
include this in the testing here.
Sam
--
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 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi
Am 29.07.21 um 21:18 schrieb Thomas Zimmermann:
Hi
Am 28.07.21 um 22:11 schrieb Sam Ravnborg:
Hi Dan,
I think I got it - we need to set irq_enabled to true.
The documentation says so:
"
* @irq_enabled:
*
* Indicates that interrupt handling is enabled,
specifically vblank
* handling. Drivers which don't use drm_irq_install()
need to set this
* to true manually.
"
Can you try to add the following line:
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev,
unsigned int irq)
+{
+ int ret;
+
+ if (irq == IRQ_NOTCONNECTED)
+ return -ENOTCONN;
+
dev->irq_enabled = true; <= THIS LINE
+ atmel_hlcdc_dc_irq_disable(dev);
+
+ ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
+ if (ret)
+ return ret;
I hope this fixes it.
It does! With the irq_enabled line added everything is looking good.
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently
replaced by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in
VBLANK ioctls").
For reference, the cover letter lists
base-commit: 2bda1ca4d4acb4892556fec3a7ea1f02afcd40bb
which is a recent drm-tip created on July 25.
Best regards
Thomas
Best regards
Thomas
Great, thanks for testing.
Thomas - I assume you will do a re-spin and there is likely some fixes
for the applied IRQ conversions too.
Note - irq_enabled must be cleared if request_irq fails. I did not
include this in the testing here.
Sam
--
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 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi
Am 28.07.21 um 22:11 schrieb Sam Ravnborg:
Hi Dan,
I think I got it - we need to set irq_enabled to true.
The documentation says so:
"
* @irq_enabled:
*
* Indicates that interrupt handling is enabled, specifically vblank
* handling. Drivers which don't use drm_irq_install() need to set
this
* to true manually.
"
Can you try to add the following line:
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int irq)
+{
+ int ret;
+
+ if (irq == IRQ_NOTCONNECTED)
+ return -ENOTCONN;
+
dev->irq_enabled = true;<= THIS LINE
+ atmel_hlcdc_dc_irq_disable(dev);
+
+ ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
+ if (ret)
+ return ret;
I hope this fixes it.
It does! With the irq_enabled line added everything is looking good.
Are you sure, you're testing with the latest drm-misc-next or drm-tip?
Because using irq_enabled is deprecated and the flag was recently
replaced by commit 1e4cd78ed493 ("drm: Don't test for IRQ support in
VBLANK ioctls").
Best regards
Thomas
Great, thanks for testing.
Thomas - I assume you will do a re-spin and there is likely some fixes
for the applied IRQ conversions too.
Note - irq_enabled must be cleared if request_irq fails. I did not
include this in the testing here.
Sam
--
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 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Sam,
On 7/28/21 12:08 PM, Sam Ravnborg wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi Dan,
>
Just to be sure...
Can you confirm that vbltest is working OK *before* this patch?
>>>
>>> Yes, can you please verify that it regressed. If so, this would mean
>>> that the driver misses vblank interrupts with the patch applied.
>>
>> Yes, unfortunately the vbltest works before this patch, but fails after
>> this patch is applied.
>
> I think I got it - we need to set irq_enabled to true.
> The documentation says so:
> "
> * @irq_enabled:
> *
> * Indicates that interrupt handling is enabled, specifically vblank
> * handling. Drivers which don't use drm_irq_install() need to set
> this
> * to true manually.
> "
>
> Can you try to add the following line:
>
>
> +static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int
> irq)
> +{
> + int ret;
> +
> + if (irq == IRQ_NOTCONNECTED)
> + return -ENOTCONN;
> +
>
> dev->irq_enabled = true;<= THIS LINE
>
>
> + atmel_hlcdc_dc_irq_disable(dev);
> +
> + ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
> dev->driver->name, dev);
> + if (ret)
> + return ret;
>
> I hope this fixes it.
It does! With the irq_enabled line added everything is looking good.
Best Regards,
Dan
>
> Sam
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Thomas, On 7/28/21 11:17 AM, Thomas Zimmermann wrote: > Hi > > Am 28.07.21 um 20:11 schrieb Sam Ravnborg: >> Hi Dan, >> >> thanks for the quick feedback! >> >> On Wed, Jul 28, 2021 at 05:50:34PM +, dan.sned...@microchip.com >> wrote: >>> On 7/28/21 8:44 AM, Sam Ravnborg wrote: EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe Hi Dan, On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: > On 7/28/21 7:00 AM, Sam Ravnborg wrote: >> [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from time to time) Sam So I ran the test on an A5D27 XULT board with a PDA5 display. Our >>> graphics demos that come with our linux4sam releases seem to run just >>> fine. modetest -v seems to run just fine. However, vbltest returns >>> "drmWaitVBlank (relative) failed ret: -1". I don't understand why >>> modetest -v is working and vbltest isn't, but that's what I'm seeing. > > Thanks for testing. > >> >> Strange indeed. >> >> >> Just to be sure... >> Can you confirm that vbltest is working OK *before* this patch? > > Yes, can you please verify that it regressed. If so, this would mean > that the driver misses vblank interrupts with the patch applied. Yes, unfortunately the vbltest works before this patch, but fails after this patch is applied. Best Regards, Dan > > Best regards > Thomas > >> >> Sam >> > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
On 7/28/21 11:11 AM, Sam Ravnborg wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Dan, > > thanks for the quick feedback! > > On Wed, Jul 28, 2021 at 05:50:34PM +, dan.sned...@microchip.com wrote: >> On 7/28/21 8:44 AM, Sam Ravnborg wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the >>> content is safe >>> >>> Hi Dan, >>> >>> On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: On 7/28/21 7:00 AM, Sam Ravnborg wrote: > [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from >>> time to time) >>> >>> Sam >>> So I ran the test on an A5D27 XULT board with a PDA5 display. Our >> graphics demos that come with our linux4sam releases seem to run just >> fine. modetest -v seems to run just fine. However, vbltest returns >> "drmWaitVBlank (relative) failed ret: -1". I don't understand why >> modetest -v is working and vbltest isn't, but that's what I'm seeing. > > Strange indeed. > > > Just to be sure... > Can you confirm that vbltest is working OK *before* this patch? > > Sam > I'm afraid vbltest works without the patch, but not with it. Dan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
On 7/28/21 8:44 AM, Sam Ravnborg wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Hi Dan, > > On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: >> On 7/28/21 7:00 AM, Sam Ravnborg wrote: >>> [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from > time to time) > > Sam > So I ran the test on an A5D27 XULT board with a PDA5 display. Our graphics demos that come with our linux4sam releases seem to run just fine. modetest -v seems to run just fine. However, vbltest returns "drmWaitVBlank (relative) failed ret: -1". I don't understand why modetest -v is working and vbltest isn't, but that's what I'm seeing. Thanks, Dan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
On 7/28/21 7:00 AM, Sam Ravnborg wrote:
> [You don't often get email from s...@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
>
> 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
>> ---
>> 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
>> #include
>> #include
>> -#include
>> #include
>> #include
>>
>> @@ -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, 0x);
>> + regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_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
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Dan,
> >
> > I think I got it - we need to set irq_enabled to true.
> > The documentation says so:
> > "
> > * @irq_enabled:
> > *
> > * Indicates that interrupt handling is enabled, specifically
> > vblank
> > * handling. Drivers which don't use drm_irq_install() need to set
> > this
> > * to true manually.
> > "
> >
> > Can you try to add the following line:
> >
> >
> > +static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int
> > irq)
> > +{
> > + int ret;
> > +
> > + if (irq == IRQ_NOTCONNECTED)
> > + return -ENOTCONN;
> > +
> >
> > dev->irq_enabled = true;<= THIS LINE
> >
> >
> > + atmel_hlcdc_dc_irq_disable(dev);
> > +
> > + ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
> > dev->driver->name, dev);
> > + if (ret)
> > + return ret;
> >
> > I hope this fixes it.
>
> It does! With the irq_enabled line added everything is looking good.
Great, thanks for testing.
Thomas - I assume you will do a re-spin and there is likely some fixes
for the applied IRQ conversions too.
Note - irq_enabled must be cleared if request_irq fails. I did not
include this in the testing here.
Sam
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Dan,
> >>
> >> Just to be sure...
> >> Can you confirm that vbltest is working OK *before* this patch?
> >
> > Yes, can you please verify that it regressed. If so, this would mean
> > that the driver misses vblank interrupts with the patch applied.
>
> Yes, unfortunately the vbltest works before this patch, but fails after
> this patch is applied.
I think I got it - we need to set irq_enabled to true.
The documentation says so:
"
* @irq_enabled:
*
* Indicates that interrupt handling is enabled, specifically vblank
* handling. Drivers which don't use drm_irq_install() need to set this
* to true manually.
"
Can you try to add the following line:
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int irq)
+{
+ int ret;
+
+ if (irq == IRQ_NOTCONNECTED)
+ return -ENOTCONN;
+
dev->irq_enabled = true;<= THIS LINE
+ atmel_hlcdc_dc_irq_disable(dev);
+
+ ret = request_irq(irq, atmel_hlcdc_dc_irq_handler, 0,
dev->driver->name, dev);
+ if (ret)
+ return ret;
I hope this fixes it.
Sam
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Am 28.07.21 um 20:11 schrieb Sam Ravnborg: Hi Dan, thanks for the quick feedback! On Wed, Jul 28, 2021 at 05:50:34PM +, dan.sned...@microchip.com wrote: On 7/28/21 8:44 AM, Sam Ravnborg wrote: EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe Hi Dan, On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: On 7/28/21 7:00 AM, Sam Ravnborg wrote: [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from time to time) Sam So I ran the test on an A5D27 XULT board with a PDA5 display. Our graphics demos that come with our linux4sam releases seem to run just fine. modetest -v seems to run just fine. However, vbltest returns "drmWaitVBlank (relative) failed ret: -1". I don't understand why modetest -v is working and vbltest isn't, but that's what I'm seeing. Thanks for testing. Strange indeed. Just to be sure... Can you confirm that vbltest is working OK *before* this patch? Yes, can you please verify that it regressed. If so, this would mean that the driver misses vblank interrupts with the patch applied. Best regards Thomas Sam -- 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 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Dan, thanks for the quick feedback! On Wed, Jul 28, 2021 at 05:50:34PM +, dan.sned...@microchip.com wrote: > On 7/28/21 8:44 AM, Sam Ravnborg wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > Hi Dan, > > > > On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: > >> On 7/28/21 7:00 AM, Sam Ravnborg wrote: > >>> [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from > > time to time) > > > > Sam > > So I ran the test on an A5D27 XULT board with a PDA5 display. Our > graphics demos that come with our linux4sam releases seem to run just > fine. modetest -v seems to run just fine. However, vbltest returns > "drmWaitVBlank (relative) failed ret: -1". I don't understand why > modetest -v is working and vbltest isn't, but that's what I'm seeing. Strange indeed. Just to be sure... Can you confirm that vbltest is working OK *before* this patch? Sam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
Hi Dan, On Wed, Jul 28, 2021 at 03:11:08PM +, dan.sned...@microchip.com wrote: > On 7/28/21 7:00 AM, Sam Ravnborg wrote: > > [You don't often get email from s...@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, crossing my fingers... (which explains the funny spelling from time to time) Sam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
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.
Hi Thomas,
IRQ_NOTCONNECTED check seems redundant, as mentioned in another patch
already.
With that considered:
Reviewed-by: Sam Ravnborg
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
> ---
> 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
> #include
> #include
> -#include
> #include
> #include
>
> @@ -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, 0x);
> + regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_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, 0x);
> - regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_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 =
[PATCH 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces
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
---
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
#include
#include
-#include
#include
#include
@@ -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, 0x);
+ regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_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, 0x);
- regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_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 = ,
.name = "atmel-hlcdc",
--
2.32.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
