On Fri, Jan 12, 2018 at 3:08 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
> On Fri, Jan 12, 2018 at 4:48 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
>> [Fair warning: This is pure conjecture right now.]
>>
>> In
>>
>> commit b8e2b0199cc377617dc238f5106352c06dcd3fa2
>> Author: Peter Rosin <peda@xxxxxxxxxx>
>> Date: Tue Jul 4 12:36:57 2017 +0200
>>
>> drm/fb-helper: factor out pseudo-palette
>>
>> Peter extracted the pseudo palette computation, but seems to have done
>> an off-by-one. I spotted that +1, but then noticed that we've passed
>> start++ to (now gone) setcolreg function, so it seemed to all match
>> up. Except post vs. pre-increment ftw.
>>
>> Result is that the palette is off-by-one, and the forground color
>> (slot 0) ends up black, rendering a fairly unreadable console.
>>
>> What baffles me is that on some systems it still seems to work
>> somehow, which lead us all down a wild goose chase trying to add
>> load_lut calls back in in various places (which was also intentionally
>> removed, but really doesn't seem to be the real root cause).
>>
>> Fixes: b8e2b0199cc3 ("drm/fb-helper: factor out pseudo-palette")
>> Cc: Peter Rosin <peda@xxxxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
>> Cc: Gustavo Padovan <gustavo@xxxxxxxxxxx>
>> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
>> Cc: David Airlie <airlied@xxxxxxxx>
>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> Cc: <stable@xxxxxxxxxxxxxxx> # v4.14+
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198123
>> Reported-by: Deposite Pirate <dpirate@xxxxxxxxxxxxxxx>
>> Reported-by: Bill Fraser <bill.fraser@xxxxxxxxx>
>> Cc: Deposite Pirate <dpirate@xxxxxxxxxxxxxxx>
>> Cc: Bill Fraser <bill.fraser@xxxxxxxxx>
>> Cc: Michel Dänzer <michel@xxxxxxxxxxx>
>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 035784ddd133..1c3a200c4a10 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1295,7 +1295,7 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>> mask <<= info->var.transp.offset;
>> value |= mask;
>> }
>> - palette[cmap->start + i] = value;
>> + palette[cmap->start] = value;
>
> I don't think this is equivalent to what was there before. Before
> there we set palette[cmap->start]->palette[cmap->start + cmap->len -
> 1], now we're only setting palette[cmap->start].
>
> In the previous version (before Peter's change), we wrote
> palette[cmap->start] twice. So while there is an off-by-one bug, I
> don't think this fixes the issue.
Well this patch is complete nonsense, no idea what I was thinking ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch