2013-08-13 09:28:24

by Pali Rohár

[permalink] [raw]
Subject: [PATCH] nouveau reclocking on nv40 not working since 77145f1cbdf8d28b46ff8070ca749bad821e0774

Hello,

in commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was introduced
error which cause that on my Nvidia 6600GT card reclocking not working
anymore. There is missing assigment of return value from pll_calc to ret.

After this patch reclocking on my card working fine again. Above broken
commit was introduced in kernel 3.7, so consider backporting this patch
to older kernels too.

Signed-off-by: Pali Roh?r <[email protected]>

diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c b/drivers/gpu/drm/nouveau/nv40_pm.c
index 3af5bcd..625f80d 100644
--- a/drivers/gpu/drm/nouveau/nv40_pm.c
+++ b/drivers/gpu/drm/nouveau/nv40_pm.c
@@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32 reg, struct nvbios_pll *pll,
if (clk < pll->vco1.max_freq)
pll->vco2.max_freq = 0;

- pclk->pll_calc(pclk, pll, clk, &coef);
+ ret = pclk->pll_calc(pclk, pll, clk, &coef);
if (ret == 0)
return -ERANGE;


--
Pali Roh?r
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2013-08-15 17:46:32

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] nouveau reclocking on nv40 not working since 77145f1cbdf8d28b46ff8070ca749bad821e0774

On Tuesday 13 August 2013 11:28:01 Pali Rohár wrote:
> Hello,
>
> in commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
> introduced error which cause that on my Nvidia 6600GT card
> reclocking not working anymore. There is missing assigment of
> return value from pll_calc to ret.
>
> After this patch reclocking on my card working fine again.
> Above broken commit was introduced in kernel 3.7, so consider
> backporting this patch to older kernels too.
>
> Signed-off-by: Pali Rohár <[email protected]>
>
> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
> 100644
> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
> pll->vco2.max_freq = 0;
>
> - pclk->pll_calc(pclk, pll, clk, &coef);
> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
> if (ret == 0)
> return -ERANGE;

Martin, can you look at another problem with my graphics card?

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2013-08-15 22:22:50

by Martin Peres

[permalink] [raw]
Subject: Re: [PATCH] nouveau reclocking on nv40 not working since 77145f1cbdf8d28b46ff8070ca749bad821e0774

On 15/08/2013 13:46, Pali Rohár wrote:
> On Tuesday 13 August 2013 11:28:01 Pali Rohár wrote:
>> Hello,
>>
>> in commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
>> introduced error which cause that on my Nvidia 6600GT card
>> reclocking not working anymore. There is missing assigment of
>> return value from pll_calc to ret.
>>
>> After this patch reclocking on my card working fine again.
>> Above broken commit was introduced in kernel 3.7, so consider
>> backporting this patch to older kernels too.
>>
>> Signed-off-by: Pali Rohár <[email protected]>
>>
>> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
>> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
>> 100644
>> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
>> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
>> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
>> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
>> pll->vco2.max_freq = 0;
>>
>> - pclk->pll_calc(pclk, pll, clk, &coef);
>> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
>> if (ret == 0)
>> return -ERANGE;
> Martin, can you look at another problem with my graphics card?
As I told you before, I'm away from my computers, so I cannot test the
patch. However,
this one seems quite obvious and should be pushed. Thanks.

2013-08-16 07:16:13

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] nouveau reclocking on nv40 not working since 77145f1cbdf8d28b46ff8070ca749bad821e0774

On Thursday 15 August 2013 18:23:06 Martin Peres wrote:
> On 15/08/2013 13:46, Pali Roh?r wrote:
> > On Tuesday 13 August 2013 11:28:01 Pali Roh?r wrote:
> >> Hello,
> >>
> >> in commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
> >> introduced error which cause that on my Nvidia 6600GT card
> >> reclocking not working anymore. There is missing assigment of
> >> return value from pll_calc to ret.
> >>
> >> After this patch reclocking on my card working fine again.
> >> Above broken commit was introduced in kernel 3.7, so consider
> >> backporting this patch to older kernels too.
> >>
> >> Signed-off-by: Pali Roh?r <[email protected]>
> >>
> >> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
> >> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
> >> 100644
> >> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
> >> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
> >> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
> >> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
> >>
> >> pll->vco2.max_freq = 0;
> >>
> >> - pclk->pll_calc(pclk, pll, clk, &coef);
> >> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
> >>
> >> if (ret == 0)
> >>
> >> return -ERANGE;
> >
> > Martin, can you look at another problem with my graphics card?
>
> As I told you before, I'm away from my computers, so I cannot test the
> patch. However,
> this one seems quite obvious and should be pushed. Thanks.

Yes, look at that diff of that problematic commit and you will see that ret
here is missing. It is possible to push this patch to 3.11?

--
Pali Roh?r
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2013-08-16 10:23:25

by Martin Peres

[permalink] [raw]
Subject: Re: [PATCH] nouveau reclocking on nv40 not working since 77145f1cbdf8d28b46ff8070ca749bad821e0774

On 16/08/2013 03:16, Pali Roh?r wrote:
> On Thursday 15 August 2013 18:23:06 Martin Peres wrote:
>> On 15/08/2013 13:46, Pali Roh?r wrote:
>>> On Tuesday 13 August 2013 11:28:01 Pali Roh?r wrote:
>>>> Hello,
>>>>
>>>> in commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
>>>> introduced error which cause that on my Nvidia 6600GT card
>>>> reclocking not working anymore. There is missing assigment of
>>>> return value from pll_calc to ret.
>>>>
>>>> After this patch reclocking on my card working fine again.
>>>> Above broken commit was introduced in kernel 3.7, so consider
>>>> backporting this patch to older kernels too.
>>>>
>>>> Signed-off-by: Pali Roh?r <[email protected]>
>>>>
>>>> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
>>>> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
>>>> 100644
>>>> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
>>>> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
>>>> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
>>>> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
>>>>
>>>> pll->vco2.max_freq = 0;
>>>>
>>>> - pclk->pll_calc(pclk, pll, clk, &coef);
>>>> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
>>>>
>>>> if (ret == 0)
>>>>
>>>> return -ERANGE;
>>> Martin, can you look at another problem with my graphics card?
>> As I told you before, I'm away from my computers, so I cannot test the
>> patch. However,
>> this one seems quite obvious and should be pushed. Thanks.
> Yes, look at that diff of that problematic commit and you will see that ret
> here is missing. It is possible to push this patch to 3.11?
Please make a proper patch (generated by git format-patch or git
send-email) and
Ben will push it.

You add my Signed-off-by: Martin Peres <[email protected]>

Cheers

2013-08-16 12:58:09

by Pali Rohár

[permalink] [raw]
Subject: [PATCH] nouveau: fix reclocking on nv40

In commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was introduced
error which cause that reclocking on nv40 not working anymore.
There is missing assigment of return value from pll_calc to ret.

Signed-off-by: Pali Rohár <[email protected]>
Signed-off-by: Martin Peres <[email protected]>
---
drivers/gpu/drm/nouveau/nv40_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c b/drivers/gpu/drm/nouveau/nv40_pm.c
index 3af5bcd..625f80d 100644
--- a/drivers/gpu/drm/nouveau/nv40_pm.c
+++ b/drivers/gpu/drm/nouveau/nv40_pm.c
@@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32 reg, struct nvbios_pll *pll,
if (clk < pll->vco1.max_freq)
pll->vco2.max_freq = 0;

- pclk->pll_calc(pclk, pll, clk, &coef);
+ ret = pclk->pll_calc(pclk, pll, clk, &coef);
if (ret == 0)
return -ERANGE;

--
1.7.10.4

2013-08-19 06:59:39

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] nouveau: fix reclocking on nv40

On Friday 16 August 2013 14:57:07 Pali Rohár wrote:
> In commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
> introduced error which cause that reclocking on nv40 not
> working anymore. There is missing assigment of return value
> from pll_calc to ret.
>
> Signed-off-by: Pali Rohár <[email protected]>
> Signed-off-by: Martin Peres <[email protected]>
> ---
> drivers/gpu/drm/nouveau/nv40_pm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
> 100644
> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
> pll->vco2.max_freq = 0;
>
> - pclk->pll_calc(pclk, pll, clk, &coef);
> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
> if (ret == 0)
> return -ERANGE;

Hello, it is possible to include this patch in 3.11?
Or it is too late now and need to wait for 3.12?

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2013-08-21 00:24:03

by Ben Skeggs

[permalink] [raw]
Subject: Re: [PATCH] nouveau: fix reclocking on nv40

On Mon, Aug 19, 2013 at 4:59 PM, Pali Roh?r <[email protected]> wrote:
> On Friday 16 August 2013 14:57:07 Pali Roh?r wrote:
>> In commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
>> introduced error which cause that reclocking on nv40 not
>> working anymore. There is missing assigment of return value
>> from pll_calc to ret.
>>
>> Signed-off-by: Pali Roh?r <[email protected]>
>> Signed-off-by: Martin Peres <[email protected]>
>> ---
>> drivers/gpu/drm/nouveau/nv40_pm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
>> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
>> 100644
>> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
>> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
>> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev, u32
>> reg, struct nvbios_pll *pll, if (clk < pll->vco1.max_freq)
>> pll->vco2.max_freq = 0;
>>
>> - pclk->pll_calc(pclk, pll, clk, &coef);
>> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
>> if (ret == 0)
>> return -ERANGE;
>
> Hello, it is possible to include this patch in 3.11?
> Or it is too late now and need to wait for 3.12?
I've picked up the patch and will submit it in my next 3.11-fixes pull request.

Thanks,
Ben.

>
> --
> Pali Roh?r
> [email protected]
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

2013-09-08 11:25:40

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] nouveau: fix reclocking on nv40

On Wednesday 21 August 2013 02:24:01 Ben Skeggs wrote:
> On Mon, Aug 19, 2013 at 4:59 PM, Pali Rohár
<[email protected]> wrote:
> > On Friday 16 August 2013 14:57:07 Pali Rohár wrote:
> >> In commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
> >> introduced error which cause that reclocking on nv40 not
> >> working anymore. There is missing assigment of return value
> >> from pll_calc to ret.
> >>
> >> Signed-off-by: Pali Rohár <[email protected]>
> >> Signed-off-by: Martin Peres <[email protected]>
> >> ---
> >>
> >> drivers/gpu/drm/nouveau/nv40_pm.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
> >> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
> >> 100644
> >> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
> >> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
> >> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev,
> >> u32 reg, struct nvbios_pll *pll, if (clk <
> >> pll->vco1.max_freq)
> >>
> >> pll->vco2.max_freq = 0;
> >>
> >> - pclk->pll_calc(pclk, pll, clk, &coef);
> >> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
> >>
> >> if (ret == 0)
> >>
> >> return -ERANGE;
> >
> > Hello, it is possible to include this patch in 3.11?
> > Or it is too late now and need to wait for 3.12?
>
> I've picked up the patch and will submit it in my next
> 3.11-fixes pull request.
>
> Thanks,
> Ben.
>

Hello, now I see that patch is in 3.11, thanks! Ben, what do you
think, can be this patch backported to older kernels?

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-09 04:28:15

by Ben Skeggs

[permalink] [raw]
Subject: Re: [PATCH] nouveau: fix reclocking on nv40

On Sun, Sep 8, 2013 at 9:25 PM, Pali Roh?r <[email protected]> wrote:
> On Wednesday 21 August 2013 02:24:01 Ben Skeggs wrote:
>> On Mon, Aug 19, 2013 at 4:59 PM, Pali Roh?r
> <[email protected]> wrote:
>> > On Friday 16 August 2013 14:57:07 Pali Roh?r wrote:
>> >> In commit 77145f1cbdf8d28b46ff8070ca749bad821e0774 was
>> >> introduced error which cause that reclocking on nv40 not
>> >> working anymore. There is missing assigment of return value
>> >> from pll_calc to ret.
>> >>
>> >> Signed-off-by: Pali Roh?r <[email protected]>
>> >> Signed-off-by: Martin Peres <[email protected]>
>> >> ---
>> >>
>> >> drivers/gpu/drm/nouveau/nv40_pm.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/nouveau/nv40_pm.c
>> >> b/drivers/gpu/drm/nouveau/nv40_pm.c index 3af5bcd..625f80d
>> >> 100644
>> >> --- a/drivers/gpu/drm/nouveau/nv40_pm.c
>> >> +++ b/drivers/gpu/drm/nouveau/nv40_pm.c
>> >> @@ -131,7 +131,7 @@ nv40_calc_pll(struct drm_device *dev,
>> >> u32 reg, struct nvbios_pll *pll, if (clk <
>> >> pll->vco1.max_freq)
>> >>
>> >> pll->vco2.max_freq = 0;
>> >>
>> >> - pclk->pll_calc(pclk, pll, clk, &coef);
>> >> + ret = pclk->pll_calc(pclk, pll, clk, &coef);
>> >>
>> >> if (ret == 0)
>> >>
>> >> return -ERANGE;
>> >
>> > Hello, it is possible to include this patch in 3.11?
>> > Or it is too late now and need to wait for 3.12?
>>
>> I've picked up the patch and will submit it in my next
>> 3.11-fixes pull request.
>>
>> Thanks,
>> Ben.
>>
>
> Hello, now I see that patch is in 3.11, thanks! Ben, what do you
> think, can be this patch backported to older kernels?
Personally, I don't care at all. The current PM code is a dead end,
and completely not "supported" (hence why it's hidden behind a magic
parameter). If I had my way it'd have been completely ripped out
already.

If someone wants to backport and test it on earlier kernels though, by
all means, go ahead :)

Thanks,
Ben.

>
> --
> Pali Roh?r
> [email protected]