2010/2/16 Jaime Velasco Juan <jsagarri...@gmail.com>:
> El lun. 15 de feb. de 2010, a las 18:52:46 +0100, Rafał Miłecki escribió:
>> 2010/2/15 Jaime Velasco Juan <jsagarri...@gmail.com>:
>> > The old code used a false condition so it always waited until
>> > timeout
>> >
>> > Signed-off-by: Jaime Velasco Juan <jsagarri...@gmail.com>
>> > ---
>> > drivers/gpu/drm/radeon/radeon_pm.c | 10 ++++++----
>> > 1 files changed, 6 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
>> > b/drivers/gpu/drm/radeon/radeon_pm.c
>> > index a8e151e..842952f 100644
>> > --- a/drivers/gpu/drm/radeon/radeon_pm.c
>> > +++ b/drivers/gpu/drm/radeon/radeon_pm.c
>> > @@ -337,10 +337,12 @@ static void radeon_pm_set_clocks(struct
>> > radeon_device *rdev)
>> > rdev->pm.req_vblank |= (1 << 1);
>> > drm_vblank_get(rdev->ddev, 1);
>> > }
>> > - if (rdev->pm.active_crtcs)
>> > - wait_event_interruptible_timeout(
>> > - rdev->irq.vblank_queue, 0,
>> > - msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT));
>> > + if (rdev->pm.active_crtcs) {
>> > + long timeout =
>> > msecs_to_jiffies(RADEON_WAIT_VBLANK_TIMEOUT);
>> > + __wait_event_interruptible_timeout(
>> > + rdev->irq.vblank_queue, 1,
>> > + timeout);
>> > + }
>>
>> Yeah, it seems logic was wrong, thanks.
>>
>> Two questions: is there some real reason for using "timeout" variable?
>> We don't re-use this anywhere. It seems useless, doesn't make code
>> shorter, forces braces usage and you defined this inside braces, not
>> on beginning of function, as kernel coding style says. Can you just
>> drop this redundant variable?
>>
>> Second: could you say what is "better" in using
>> __wait_event_interruptible_timeout? Is this faster (doesn't check
>> condition) or something?
>>
> I just took the code from the definition of wait_event_interruptible_timeout;
> that macro checks the condition before calling
> __wait_event_interruptible_timeout, where the task really goes to sleep,
> but __wait_event_interruptible_timeout changes the timeout parameter, so
> it needs to be a variable. I just tried to do the smallest change to the
> code. It's true that almost nobody else use this macro directly.
>
> Even with this, I still see some ocasional glitches when reclocking the
> engine, but this does not stall rendering for 200ms as before (it was
> very noticeable). This has nothing to do with this patch, but is it really
> needed to take cp.mutex in radeon_pm_set_clocks? I've been doing some
> testing without doing it and it doesn't seem to break anything.
>
You generally don't want to change the clock while the engine using it
is active.
Alex
> Regards
>
>> --
>> Rafał
------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
