Christian Costa
2014-Aug-25 19:00 UTC
[Nouveau] [PATCH envytools] Fix range end to the last value of timing table.
--- nva/set_timings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nva/set_timings.c b/nva/set_timings.c index 7376486..985a707 100644 --- a/nva/set_timings.c +++ b/nva/set_timings.c @@ -506,7 +506,7 @@ shallow_dump(struct nvamemtiming_conf *conf) if (conf->range.start == (unsigned char) -1) conf->range.start = 0; if (conf->range.end == (unsigned char) -1) - conf->range.end = conf->vbios.timing_entry_length; + conf->range.end = conf->vbios.timing_entry_length-1; fprintf(stderr, "Shallow mode: Will iterate between %i and %i\n", conf->range.start, conf->range.end); -- 1.9.1
Martin Peres
2014-Aug-28 08:27 UTC
[Nouveau] [PATCH envytools] Fix range end to the last value of timing table.
On 25/08/2014 21:00, Christian Costa wrote:> --- > nva/set_timings.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/nva/set_timings.c b/nva/set_timings.c > index 7376486..985a707 100644 > --- a/nva/set_timings.c > +++ b/nva/set_timings.c > @@ -506,7 +506,7 @@ shallow_dump(struct nvamemtiming_conf *conf) > if (conf->range.start == (unsigned char) -1) > conf->range.start = 0; > if (conf->range.end == (unsigned char) -1) > - conf->range.end = conf->vbios.timing_entry_length; > + conf->range.end = conf->vbios.timing_entry_length-1; > > fprintf(stderr, "Shallow mode: Will iterate between %i and %i\n", conf->range.start, conf->range.end); >Hey I don't like this patch because it would create an output difference between the deep and the shallow test. Please check and propose another patch. What are you trying to fix by the way? You don't like that range.end is not included?
Christian Costa
2014-Aug-29 02:25 UTC
[Nouveau] [PATCH envytools] Fix range end to the last value of timing table.
Le 28/08/2014 10:27, Martin Peres a ?crit :> On 25/08/2014 21:00, Christian Costa wrote: >> --- >> nva/set_timings.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/nva/set_timings.c b/nva/set_timings.c >> index 7376486..985a707 100644 >> --- a/nva/set_timings.c >> +++ b/nva/set_timings.c >> @@ -506,7 +506,7 @@ shallow_dump(struct nvamemtiming_conf *conf) >> if (conf->range.start == (unsigned char) -1) >> conf->range.start = 0; >> if (conf->range.end == (unsigned char) -1) >> - conf->range.end = conf->vbios.timing_entry_length; >> + conf->range.end = conf->vbios.timing_entry_length-1; >> fprintf(stderr, "Shallow mode: Will iterate between %i and >> %i\n", conf->range.start, conf->range.end); > Hey > > I don't like this patch because it would create an output difference > between the deep and the shallow test. > Please check and propose another patch.Hi Martin, Yes. You're right. I didn't pay attention to deep_dump.> > > What are you trying to fix by the way? You don't like that range.end > is not included? >I got confused with the message and tought there was an out of the bound access. Looking more closely at the code says indeed that range.end is actually not included. It doesn't seem straitforward. At least to me. I would change the code to make range.end included to avoid confusion. What do you think? By the way, in deep_mode, crange.start and crange.end are calculated but not used at all. The loop iterates the whole entry: for (i = 0; i < conf->vbios.timing_entry_length; i++) Is it normal ? While I am at it, I tried to understand what deep_mode does. Contrary to shallow_mode which tests the table value + 1, deep_mode tests all values between values of 2 entries. The code is as follows: for (v = initial+1; v <= target; v++) { conf->vbios.data[conf->vbios.timing_entry_offset + index] = v; launch(conf, outf, index + 1 , COLOR); } It works fine when initial value is lower than the target but does nothing in the other case. Looks like a bug, no? Thanks Christian
Apparently Analagous Threads
- [PATCH envytools] nvbios: Fix reading of ram_restrict_group_count.
- [PATCH envytools] Fix range end to the last value of timing table.
- [PATCH envytools] nvbios: Add missing null byte to string read from file.
- [PATCH envytools] nvamemtiming: Handle target < initial case when iterating values
- [PATCH envytools] nva: Clean up nva tools doc