[PATCH 0/6] drm: avoid poking at dev->vblank[] directly

[PATCH 0/6] drm: avoid poking at dev->vblank[] directly

[Jani Nikula]

We have accessors for dev->vblank[], and poking at that directly should
be reserved for drm_vblank.c. This series along with [1] switches to use
the accessors.

I've got a whole another series brewing refactoring drm_vblank.c, but
wanted to split this out of the series to keep it manageable. None of
the patches here depend on each other, and the refactoring doesn't
depend on it, so the patches to drivers can be merged every which way,
just let me know.

BR,
Jani.



[1] https://lore.kernel.org/r/20251106200000.1455164-1-jani.nikula@intel.com


Jani Nikula (6):
  drm/vblank: use drm_crtc_vblank_crtc() in workers
  drm/atomic: use drm_crtc_vblank_waitqueue()
  drm/msm: use drm_crtc_vblank_waitqueue()
  drm/tidss: use drm_crtc_vblank_crtc()
  drm/vmwgfx: use drm_crtc_vblank_crtc()
  drm/gma500: use drm_crtc_vblank_crtc()

 drivers/gpu/drm/drm_atomic_helper.c       |  4 ++-
 drivers/gpu/drm/drm_vblank_work.c         |  2 +-
 drivers/gpu/drm/gma500/psb_irq.c          | 36 +++++++++++++++--------
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  3 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c |  3 +-
 drivers/gpu/drm/tidss/tidss_crtc.c        |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c      |  6 ++--
 7 files changed, 34 insertions(+), 23 deletions(-)

-- 
2.47.3

[PATCH 1/6] drm/vblank: use drm_crtc_vblank_crtc() in workers

[Jani Nikula]

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.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_vblank_work.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
index e4e1873f0e1e..70f0199251ea 100644
--- a/drivers/gpu/drm/drm_vblank_work.c
+++ b/drivers/gpu/drm/drm_vblank_work.c
@@ -244,7 +244,7 @@ EXPORT_SYMBOL(drm_vblank_work_flush);
 void drm_vblank_work_flush_all(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)];
+	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
 	spin_lock_irq(&dev->event_lock);
 	wait_event_lock_irq(vblank->work_wait_queue,
-- 
2.47.3

Re: [PATCH 1/6] drm/vblank: use drm_crtc_vblank_crtc() in workers

[Thomas Zimmermann]

Am 07.11.25 um 12:04 schrieb Jani Nikula:
> 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.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/drm_vblank_work.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
> index e4e1873f0e1e..70f0199251ea 100644
> --- a/drivers/gpu/drm/drm_vblank_work.c
> +++ b/drivers/gpu/drm/drm_vblank_work.c
> @@ -244,7 +244,7 @@ EXPORT_SYMBOL(drm_vblank_work_flush);
>   void drm_vblank_work_flush_all(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
> -	struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)];
> +	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>   
>   	spin_lock_irq(&dev->event_lock);
>   	wait_event_lock_irq(vblank->work_wait_queue,

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

[PATCH 2/6] drm/atomic: use drm_crtc_vblank_waitqueue()

[Jani Nikula]

We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
for a vblank. Use it instead of poking at dev->vblank[] directly.

Due to the macro maze of wait_event_timeout() that uses the address-of
operator on the argument, we have to pass it in with the indirection
operator.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5a473a274ff0..e641fcf8c568 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1831,10 +1831,12 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 	}
 
 	for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
+		wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
+
 		if (!(crtc_mask & drm_crtc_mask(crtc)))
 			continue;
 
-		ret = wait_event_timeout(dev->vblank[i].queue,
+		ret = wait_event_timeout(*queue,
 					 state->crtcs[i].last_vblank_count !=
 						drm_crtc_vblank_count(crtc),
 					 msecs_to_jiffies(100));
-- 
2.47.3

Re: [PATCH 2/6] drm/atomic: use drm_crtc_vblank_waitqueue()

[Thomas Zimmermann]

Hi

Am 07.11.25 um 12:04 schrieb Jani Nikula:
> We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
> for a vblank. Use it instead of poking at dev->vblank[] directly.
>
> Due to the macro maze of wait_event_timeout() that uses the address-of
> operator on the argument, we have to pass it in with the indirection
> operator.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by Thomas Zimmermann <tzimmermann@suse.de>

But... drm_crtc_vblank_waitqueue() is a terrible interface IMHO, as it 
exports internal details of the vblank implementation.

I wonder if the existing users at [1] and [2] couldn't be replaced with 
a common vblank helper.

And there's drm_wait_one_vblank() [3] and the waiting that's being fixed 
here [4]. The latter looks like [3] but with multiple CRTC waiting for 
their next vblank. I'd say this could be a single implementation within 
the vblank code.

[1] 
https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_display_rps.c#L73
[2] 
https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_vblank.c#L715
[3] 
https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_vblank.c#L1304
[4] 
https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_atomic_helper.c#L1837

Best regards
Thomas

> ---
>   drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 5a473a274ff0..e641fcf8c568 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1831,10 +1831,12 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>   	}
>   
>   	for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
> +		wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
> +
>   		if (!(crtc_mask & drm_crtc_mask(crtc)))
>   			continue;
>   
> -		ret = wait_event_timeout(dev->vblank[i].queue,
> +		ret = wait_event_timeout(*queue,
>   					 state->crtcs[i].last_vblank_count !=
>   						drm_crtc_vblank_count(crtc),
>   					 msecs_to_jiffies(100));

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

Re: [PATCH 2/6] drm/atomic: use drm_crtc_vblank_waitqueue()

[Jani Nikula]

On Mon, 10 Nov 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 07.11.25 um 12:04 schrieb Jani Nikula:
>> We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
>> for a vblank. Use it instead of poking at dev->vblank[] directly.
>>
>> Due to the macro maze of wait_event_timeout() that uses the address-of
>> operator on the argument, we have to pass it in with the indirection
>> operator.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by Thomas Zimmermann <tzimmermann@suse.de>
>
> But... drm_crtc_vblank_waitqueue() is a terrible interface IMHO, as it 
> exports internal details of the vblank implementation.
>
> I wonder if the existing users at [1] and [2] couldn't be replaced with 
> a common vblank helper.
>
> And there's drm_wait_one_vblank() [3] and the waiting that's being fixed 
> here [4]. The latter looks like [3] but with multiple CRTC waiting for 
> their next vblank. I'd say this could be a single implementation within 
> the vblank code.

I don't disagree, but getting that done is a bit more involved than what
I have time for right now. Need to think.

In the mean time, pushed the drm_crtc_vblank_crtc() related patches in
the series, and left the drm_crtc_vblank_waitqueue() ones to simmer.

Thanks for the reviews.

BR,
Jani.


>
> [1] 
> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_display_rps.c#L73
> [2] 
> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_vblank.c#L715
> [3] 
> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_vblank.c#L1304
> [4] 
> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_atomic_helper.c#L1837
>
> Best regards
> Thomas
>
>> ---
>>   drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 5a473a274ff0..e641fcf8c568 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1831,10 +1831,12 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>>   	}
>>   
>>   	for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
>> +		wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
>> +
>>   		if (!(crtc_mask & drm_crtc_mask(crtc)))
>>   			continue;
>>   
>> -		ret = wait_event_timeout(dev->vblank[i].queue,
>> +		ret = wait_event_timeout(*queue,
>>   					 state->crtcs[i].last_vblank_count !=
>>   						drm_crtc_vblank_count(crtc),
>>   					 msecs_to_jiffies(100));

-- 
Jani Nikula, Intel

Re: [PATCH 2/6] drm/atomic: use drm_crtc_vblank_waitqueue()

[Thomas Zimmermann]

Hi

Am 10.11.25 um 13:51 schrieb Jani Nikula:
> On Mon, 10 Nov 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>> Am 07.11.25 um 12:04 schrieb Jani Nikula:
>>> We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
>>> for a vblank. Use it instead of poking at dev->vblank[] directly.
>>>
>>> Due to the macro maze of wait_event_timeout() that uses the address-of
>>> operator on the argument, we have to pass it in with the indirection
>>> operator.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Reviewed-by Thomas Zimmermann <tzimmermann@suse.de>
>>
>> But... drm_crtc_vblank_waitqueue() is a terrible interface IMHO, as it
>> exports internal details of the vblank implementation.
>>
>> I wonder if the existing users at [1] and [2] couldn't be replaced with
>> a common vblank helper.
>>
>> And there's drm_wait_one_vblank() [3] and the waiting that's being fixed
>> here [4]. The latter looks like [3] but with multiple CRTC waiting for
>> their next vblank. I'd say this could be a single implementation within
>> the vblank code.
> I don't disagree, but getting that done is a bit more involved than what
> I have time for right now. Need to think.
>
> In the mean time, pushed the drm_crtc_vblank_crtc() related patches in
> the series, and left the drm_crtc_vblank_waitqueue() ones to simmer.

Please also merge the rest of the series. These patches are an 
improvement to open-coding the access to the fields.

Best regards
Thomas

>
> Thanks for the reviews.
>
> BR,
> Jani.
>
>
>> [1]
>> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_display_rps.c#L73
>> [2]
>> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_vblank.c#L715
>> [3]
>> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_vblank.c#L1304
>> [4]
>> https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_atomic_helper.c#L1837
>>
>> Best regards
>> Thomas
>>
>>> ---
>>>    drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>> index 5a473a274ff0..e641fcf8c568 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -1831,10 +1831,12 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>>>    	}
>>>    
>>>    	for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
>>> +		wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
>>> +
>>>    		if (!(crtc_mask & drm_crtc_mask(crtc)))
>>>    			continue;
>>>    
>>> -		ret = wait_event_timeout(dev->vblank[i].queue,
>>> +		ret = wait_event_timeout(*queue,
>>>    					 state->crtcs[i].last_vblank_count !=
>>>    						drm_crtc_vblank_count(crtc),
>>>    					 msecs_to_jiffies(100));

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

Re: [PATCH 2/6] drm/atomic: use drm_crtc_vblank_waitqueue()

[Jani Nikula]

On Mon, 10 Nov 2025, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Please also merge the rest of the series. These patches are an 
> improvement to open-coding the access to the fields.

Fair. Thanks, pushed now.

BR,
Jani.


-- 
Jani Nikula, Intel

[PATCH 3/6] drm/msm: use drm_crtc_vblank_waitqueue()

[Jani Nikula]

We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
for a vblank. Use it instead of poking at dev->vblank[] directly.

Due to the macro maze of wait_event_timeout() that uses the address-of
operator on the argument, we have to pass it in with the indirection
operator.

Cc: Rob Clark <robin.clark@oss.qualcomm.com>
Cc: Dmitry Baryshkov <lumag@kernel.org>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
Cc: Jessica Zhang <jesszhan0024@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 3 ++-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index da53ca88251e..e8066f9fd534 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -527,13 +527,14 @@ static void mdp4_crtc_wait_for_flush_done(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
 	struct mdp4_kms *mdp4_kms = get_kms(crtc);
+	wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
 	int ret;
 
 	ret = drm_crtc_vblank_get(crtc);
 	if (ret)
 		return;
 
-	ret = wait_event_timeout(dev->vblank[drm_crtc_index(crtc)].queue,
+	ret = wait_event_timeout(*queue,
 		!(mdp4_read(mdp4_kms, REG_MDP4_OVERLAY_FLUSH) &
 			mdp4_crtc->flushed_mask),
 		msecs_to_jiffies(50));
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 4c4900a7beda..373ae7d9bf01 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -1234,6 +1234,7 @@ static void mdp5_crtc_wait_for_flush_done(struct drm_crtc *crtc)
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
 	struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
 	struct mdp5_ctl *ctl = mdp5_cstate->ctl;
+	wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc);
 	int ret;
 
 	/* Should not call this function if crtc is disabled. */
@@ -1244,7 +1245,7 @@ static void mdp5_crtc_wait_for_flush_done(struct drm_crtc *crtc)
 	if (ret)
 		return;
 
-	ret = wait_event_timeout(dev->vblank[drm_crtc_index(crtc)].queue,
+	ret = wait_event_timeout(*queue,
 		((mdp5_ctl_get_commit_status(ctl) &
 		mdp5_crtc->flushed_mask) == 0),
 		msecs_to_jiffies(50));
-- 
2.47.3

Re: [PATCH 3/6] drm/msm: use drm_crtc_vblank_waitqueue()

[Dmitry Baryshkov]

On Fri, Nov 07, 2025 at 01:04:57PM +0200, Jani Nikula wrote:
> We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer
> for a vblank. Use it instead of poking at dev->vblank[] directly.
> 
> Due to the macro maze of wait_event_timeout() that uses the address-of
> operator on the argument, we have to pass it in with the indirection
> operator.
> 
> Cc: Rob Clark <robin.clark@oss.qualcomm.com>
> Cc: Dmitry Baryshkov <lumag@kernel.org>
> Cc: Abhinav Kumar <abhinav.kumar@linux.dev>
> Cc: Jessica Zhang <jesszhan0024@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 3 ++-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH 4/6] drm/tidss: use drm_crtc_vblank_crtc()

[Jani Nikula]

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.

Cc: Jyri Sarha <jyri.sarha@iki.fi>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/tidss/tidss_crtc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index 411b1a25e29c..8f81eb560b9e 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -248,8 +248,7 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc,
 	dispc_vp_enable(tidss->dispc, tcrtc->hw_videoport);
 
 	if (crtc->state->event) {
-		unsigned int pipe = drm_crtc_index(crtc);
-		struct drm_vblank_crtc *vblank = &ddev->vblank[pipe];
+		struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
 		vblank->time = ktime_get();
 
-- 
2.47.3

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

[Jyri Sarha]

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

November 7, 2025 at 1:04 PM, "Jani Nikula" <jani.nikula@intel.com mailto:jani.nikula@intel.com?to=%22Jani%20Nikula%22%20%3Cjani.nikula%40intel.com%3E > 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.
> 
> Cc: Jyri Sarha <jyri.sarha@iki.fi>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
Acked-by: Jyri Sarha <jyri.sarha@iki.fi mailto:jyri.sarha@iki.fi >

Thanks,.
Jyri
---
 drivers/gpu/drm/tidss/tidss_crtc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index 411b1a25e29c..8f81eb560b9e 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -248,8 +248,7 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc,
 dispc_vp_enable(tidss->dispc, tcrtc->hw_videoport);
 
 if (crtc->state->event) {
- unsigned int pipe = drm_crtc_index(crtc);
- struct drm_vblank_crtc *vblank = &ddev->vblank[pipe];
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
 vblank->time = ktime_get();
 
-- 
2.47.3

[-- Attachment #2: Type: text/html, Size: 1629 bytes --]

[PATCH 5/6] drm/vmwgfx: use drm_crtc_vblank_crtc()

[Jani Nikula]

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.

Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
index aec774fa4d7b..5abd7f5ad2db 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
@@ -247,9 +247,8 @@ vmw_vkms_get_vblank_timestamp(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct vmw_private *vmw = vmw_priv(dev);
-	unsigned int pipe = crtc->index;
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
+	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 
 	if (!vmw->vkms_enabled)
 		return false;
@@ -281,8 +280,7 @@ vmw_vkms_enable_vblank(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct vmw_private *vmw = vmw_priv(dev);
-	unsigned int pipe = drm_crtc_index(crtc);
-	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
+	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
 	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
 
 	if (!vmw->vkms_enabled)
-- 
2.47.3

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

[Ian Forbes]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On Fri, Nov 7, 2025 at 5:05 AM 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.
>
> Cc: Zack Rusin <zack.rusin@broadcom.com>
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---

Reviewed-by: Ian Forbes <ian.forbes@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

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

[Jani Nikula]

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

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

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

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

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

[Thomas Zimmermann]

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)

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

[Jani Nikula]

On Fri, 30 Jan 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 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.

Thanks for the report. Since we're at -rc7 and there's going to be -rc8,
I think the only reasonable approach at this time is to revert [1].

BR,
Jani.


[1] https://lore.kernel.org/r/20260130151319.31264-1-jani.nikula@intel.com



-- 
Jani Nikula, Intel