Re: [PATCH 6/6] drm/gma500: use drm_crtc_vblank_crtc()

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 07.11.25 um 14:11 schrieb Patrik Jakobsson:
> On Fri, Nov 7, 2025 at 12:05 PM Jani Nikula <jani.nikula@intel.com> wrote:
>> We have drm_crtc_vblank_crtc() to get the struct drm_vblank_crtc pointer
>> for a crtc. Use it instead of poking at dev->vblank[] directly.
>>
>> However, we also need to get the crtc to start with. We could use
>> drm_crtc_from_index(), but refactor to use drm_for_each_crtc() instead.
>>
>> This is all a bit tedious, and perhaps the driver shouldn't be poking at
>> vblank->enabled directly in the first place. But at least hide away the
>> dev->vblank[] access in drm_vblank.c where it belongs.
>>
>> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Hi Jani,
> The gma500 part looks good. Feel free to merge this yourself.
>
> Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

This patch breaks the driver with a NULL-ptr oops on startup. This is 
because the IRQ initialization in gma_irq_install() now uses CRTCs that 
are only allocated later in psb_modeset_init(). Stack trace is below.

There's a nearby comment about preserving the order of the operations, 
so I don't dare touching it. But reverting commit d930ffa5d6e8 
("drm/gma500: use drm_crtc_vblank_crtc()") resolves the issue.

Best regards
Thomas


[   65.831766] Oops: general protection fault, probably for 
non-canonical address 0xdffffc0000000021: 0000 [#1] SMP KASAN NOPTI
[   65.832114] KASAN: null-ptr-deref in range 
[0x0000000000000108-0x000000000000010f]
[   65.832232] CPU: 1 UID: 0 PID: 296 Comm: (udev-worker) Tainted: G    
         E       6.19.0-rc6-1-default+ #4622 PREEMPT(voluntary)
[   65.832376] Tainted: [E]=UNSIGNED_MODULE
[   65.832448] Hardware name:  /DN2800MT, BIOS 
MTCDT10N.86A.0164.2012.1213.1024 12/13/2012
[   65.832543] RIP: 0010:drm_crtc_vblank_crtc+0x24/0xd0
[   65.832652] Code: 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 81 c7 
18 01 00 00 48 83 ec 10 48 ba 00 00 00 00 00 fc ff df 48 89 f9 48 c1 e9 
03 <0f> b6 14 11 84 d2 74 05 80 fa 03 7e 58 48 89 c6 8b 90 18 01 00
00
[   65.832820] RSP: 0018:ffff88800c8f7688 EFLAGS: 00010006
[   65.832919] RAX: fffffffffffffff0 RBX: ffff88800fff4928 RCX: 
0000000000000021
[   65.833011] RDX: dffffc0000000000 RSI: ffffc90000978130 RDI: 
0000000000000108
[   65.833107] RBP: ffffed1001ffea03 R08: 0000000000000000 R09: 
ffffed100191eec7
[   65.833199] R10: 0000000000000001 R11: 0000000000000001 R12: 
ffff8880014480c8
[   65.833289] R13: dffffc0000000000 R14: fffffffffffffff0 R15: 
ffff88800fff4000
[   65.833380] FS:  00007fe53d4d5d80(0000) GS:ffff888148dd8000(0000) 
knlGS:0000000000000000
[   65.833488] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   65.833575] CR2: 00007fac707420b8 CR3: 000000000ebd1000 CR4: 
00000000000006f0
[   65.833668] Call Trace:
[   65.833735]  <TASK>
[   65.833808]  gma_irq_preinstall+0x190/0x3e0 [gma500_gfx]
[   65.834054]  gma_irq_install+0xb2/0x240 [gma500_gfx]
[   65.834282]  psb_driver_load+0x7b2/0x1090 [gma500_gfx]
[   65.834516]  ? __pfx_psb_driver_load+0x10/0x10 [gma500_gfx]
[   65.834726]  ? ksize+0x1d/0x40
[   65.834817]  ? drmm_add_final_kfree+0x3b/0xb0
[   65.834935]  ? __pfx_psb_pci_probe+0x10/0x10 [gma500_gfx]
[   65.835164]  psb_pci_probe+0xc8/0x150 [gma500_gfx]
[   65.835384]  local_pci_probe+0xd5/0x190
[   65.835492]  pci_call_probe+0x167/0x4b0
[   65.835594]  ? __pfx_pci_call_probe+0x10/0x10
[   65.835693]  ? local_clock+0x11/0x30
[   65.835808]  ? __pfx___driver_attach+0x10/0x10
[   65.835915]  ? do_raw_spin_unlock+0x55/0x230
[   65.836014]  ? pci_match_device+0x303/0x790
[   65.836124]  ? pci_match_device+0x386/0x790
[   65.836226]  ? __pfx_pci_assign_irq+0x10/0x10
[   65.836320]  ? kernfs_create_link+0x16a/0x230
[   65.836418]  ? do_raw_spin_unlock+0x55/0x230
[   65.836526]  ? __pfx___driver_attach+0x10/0x10
[   65.836626]  pci_device_probe+0x175/0x2c0
[   65.836735]  call_driver_probe+0x64/0x1e0
[   65.836842]  really_probe+0x194/0x740
[   65.836951]  ? __pfx___driver_attach+0x10/0x10
[   65.837053]  __driver_probe_device+0x18c/0x3a0
[   65.837163]  ? __pfx___driver_attach+0x10/0x10
[   65.837262]  driver_probe_device+0x4a/0x120
[   65.837369]  __driver_attach+0x19c/0x550
[   65.837474]  ? __pfx___driver_attach+0x10/0x10
[   65.837575]  bus_for_each_dev+0xe6/0x150
[   65.837669]  ? local_clock+0x11/0x30
[   65.837770]  ? __pfx_bus_for_each_dev+0x10/0x10
[   65.837891]  bus_add_driver+0x2af/0x4f0
[   65.838000]  ? __pfx_psb_init+0x10/0x10 [gma500_gfx]
[   65.838236]  driver_register+0x19f/0x3a0
[   65.838342]  ? rcu_is_watching+0x11/0xb0
[   65.838446]  do_one_initcall+0xb5/0x3a0
[   65.838546]  ? __pfx_do_one_initcall+0x10/0x10
[   65.838644]  ? __kasan_slab_alloc+0x2c/0x70
[   65.838741]  ? rcu_is_watching+0x11/0xb0
[   65.838837]  ? __kmalloc_cache_noprof+0x3e8/0x6e0
[   65.838937]  ? klp_module_coming+0x1a0/0x2e0
[   65.839033]  ? do_init_module+0x85/0x7f0
[   65.839126]  ? kasan_unpoison+0x40/0x70
[   65.839230]  do_init_module+0x26e/0x7f0
[   65.839341]  ? __pfx_do_init_module+0x10/0x10
[   65.839450]  init_module_from_file+0x13f/0x160
[   65.839549]  ? __pfx_init_module_from_file+0x10/0x10
[   65.839651]  ? __lock_acquire+0x578/0xae0
[   65.839791]  ? do_raw_spin_unlock+0x55/0x230
[   65.839886]  ? idempotent_init_module+0x585/0x720
[   65.839993]  idempotent_init_module+0x1ff/0x720
[   65.840097]  ? __pfx_cred_has_capability.isra.0+0x10/0x10
[   65.840211]  ? __pfx_idempotent_init_module+0x10/0x10
[   65.840342]  ? [   65.844743] note: (udev-worker)[296] exited with 
preempt_count 1




>
>> ---
>>   drivers/gpu/drm/gma500/psb_irq.c | 36 ++++++++++++++++++++------------
>>   1 file changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
>> index c224c7ff353c..3a946b472064 100644
>> --- a/drivers/gpu/drm/gma500/psb_irq.c
>> +++ b/drivers/gpu/drm/gma500/psb_irq.c
>> @@ -250,6 +250,7 @@ static irqreturn_t gma_irq_handler(int irq, void *arg)
>>   void gma_irq_preinstall(struct drm_device *dev)
>>   {
>>          struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>> +       struct drm_crtc *crtc;
>>          unsigned long irqflags;
>>
>>          spin_lock_irqsave(&dev_priv->irqmask_lock, irqflags);
>> @@ -260,10 +261,15 @@ void gma_irq_preinstall(struct drm_device *dev)
>>          PSB_WSGX32(0x00000000, PSB_CR_EVENT_HOST_ENABLE);
>>          PSB_RSGX32(PSB_CR_EVENT_HOST_ENABLE);
>>
>> -       if (dev->vblank[0].enabled)
>> -               dev_priv->vdc_irq_mask |= _PSB_VSYNC_PIPEA_FLAG;
>> -       if (dev->vblank[1].enabled)
>> -               dev_priv->vdc_irq_mask |= _PSB_VSYNC_PIPEB_FLAG;
>> +       drm_for_each_crtc(crtc, dev) {
>> +               struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> +
>> +               if (vblank->enabled) {
>> +                       u32 mask = drm_crtc_index(crtc) ? _PSB_VSYNC_PIPEB_FLAG :
>> +                               _PSB_VSYNC_PIPEA_FLAG;
>> +                       dev_priv->vdc_irq_mask |= mask;
>> +               }
>> +       }
>>
>>          /* Revisit this area - want per device masks ? */
>>          if (dev_priv->ops->hotplug)
>> @@ -278,8 +284,8 @@ void gma_irq_preinstall(struct drm_device *dev)
>>   void gma_irq_postinstall(struct drm_device *dev)
>>   {
>>          struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>> +       struct drm_crtc *crtc;
>>          unsigned long irqflags;
>> -       unsigned int i;
>>
>>          spin_lock_irqsave(&dev_priv->irqmask_lock, irqflags);
>>
>> @@ -292,11 +298,13 @@ void gma_irq_postinstall(struct drm_device *dev)
>>          PSB_WVDC32(dev_priv->vdc_irq_mask, PSB_INT_ENABLE_R);
>>          PSB_WVDC32(0xFFFFFFFF, PSB_HWSTAM);
>>
>> -       for (i = 0; i < dev->num_crtcs; ++i) {
>> -               if (dev->vblank[i].enabled)
>> -                       gma_enable_pipestat(dev_priv, i, PIPE_VBLANK_INTERRUPT_ENABLE);
>> +       drm_for_each_crtc(crtc, dev) {
>> +               struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> +
>> +               if (vblank->enabled)
>> +                       gma_enable_pipestat(dev_priv, drm_crtc_index(crtc), PIPE_VBLANK_INTERRUPT_ENABLE);
>>                  else
>> -                       gma_disable_pipestat(dev_priv, i, PIPE_VBLANK_INTERRUPT_ENABLE);
>> +                       gma_disable_pipestat(dev_priv, drm_crtc_index(crtc), PIPE_VBLANK_INTERRUPT_ENABLE);
>>          }
>>
>>          if (dev_priv->ops->hotplug_enable)
>> @@ -337,8 +345,8 @@ void gma_irq_uninstall(struct drm_device *dev)
>>   {
>>          struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>>          struct pci_dev *pdev = to_pci_dev(dev->dev);
>> +       struct drm_crtc *crtc;
>>          unsigned long irqflags;
>> -       unsigned int i;
>>
>>          if (!dev_priv->irq_enabled)
>>                  return;
>> @@ -350,9 +358,11 @@ void gma_irq_uninstall(struct drm_device *dev)
>>
>>          PSB_WVDC32(0xFFFFFFFF, PSB_HWSTAM);
>>
>> -       for (i = 0; i < dev->num_crtcs; ++i) {
>> -               if (dev->vblank[i].enabled)
>> -                       gma_disable_pipestat(dev_priv, i, PIPE_VBLANK_INTERRUPT_ENABLE);
>> +       drm_for_each_crtc(crtc, dev) {
>> +               struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> +
>> +               if (vblank->enabled)
>> +                       gma_disable_pipestat(dev_priv, drm_crtc_index(crtc), PIPE_VBLANK_INTERRUPT_ENABLE);
>>          }
>>
>>          dev_priv->vdc_irq_mask &= _PSB_IRQ_SGX_FLAG |
>> --
>> 2.47.3
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)