RE: drm/ast: Linux 4.14 AST DRM driver might not load gamma table [patch proposed]

2017-12-22 Thread Carroll, Lewis
Spoke too soon. Original patch worked better (adding call to DPMS ON event
in commit call-back. Disregard the latest I sent. Apologies.

> -Original Message-
> From: Carroll, Lewis
> Sent: Thursday, December 21, 2017 3:58 PM
> To: dri-devel@lists.freedesktop.org
> Cc: Wentland, Harry <harry.wentl...@amd.com>; 'Daniel Vetter'
> <dan...@ffwll.ch>; Daenzer, Michel <michel.daen...@amd.com>
> Subject: RE: drm/ast: Linux 4.14 AST DRM driver might not load gamma table
> [patch proposed]
> 
> > -Original Message-
> > From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> > Daniel Vetter
> > Sent: Thursday, December 21, 2017 3:31 PM
> > To: Carroll, Lewis <lewis.carr...@amd.com>; Daenzer, Michel
> > <michel.daen...@amd.com>
> > Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
> > de...@lists.freedesktop.org
> > Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma
> table
> > [patch proposed]
> >
> > Fyi Michel, we've discussed similar issues for radeon/amdgpu on irc.
> >
> > On Thu, Dec 21, 2017 at 4:21 PM, Carroll, Lewis <lewis.carr...@amd.com>
> > wrote:
> > >> -Original Message-
> > >> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > >> Vetter
> > >> Sent: Thursday, December 21, 2017 5:07 AM
> > >> To: Carroll, Lewis <lewis.carr...@amd.com>
> > >> Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
> > >> de...@lists.freedesktop.org
> > >> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma
> > table
> > >> [patch proposed]
> > >>
> > >> On Thu, Dec 21, 2017 at 12:00:39AM +, Carroll, Lewis wrote:
> > >> > The discussion sounds similar as well - related to load_lut() not being
> > called.
> > >> >
> > >> > Perhaps after the drm-next-4.14 merge, whatever call stack used to
> > cause
> > >> > load_lut to always get called is now gone. Even if
> > FB_VISUAL_TRUECOLOR
> > >> > doesn't support a clut, some hardware may still need a default gamma
> > lut
> > >> > loaded (appears to be the case with the AST2500). Perhaps checking
> for
> > >> > that profile and loading the default LUT prepared by
> > >> > drm_mode_crtc_set_gamma_size when FB_VISUAL_TRUECOLOR...?
> > >>
> > >> Yeah drivers should load that somehow. Unfortunately I'm going on
> > >> vacations now, so I'm not going to fix anything anytime soon. Might be
> > >> good to ping Peter Rosin, he did all the fbdev emulation gamma
> handling
> > >> rework (which is the patch that removed the superflously-looking call to
> > >> ->load_lut that some drivers relied on to have a consistent lut state).
> > >> -Daniel
> > >
> > > Enjoy the holidays.
> > >
> > > Wonder if it would be better to just call load_lut after
> > > drm_mode_crtc_set_gamma_size instead of adding a potentially
> > > unnecessary DPMS ON event to the commit call-back as I did...? Or call
> > > load_lut in the commit callback instead of the DPMS ON call...?
> > >
> > > The first approach (calling load_lut after set_gamma_size) also works on
> > > two test systems.
> >
> > Yeah, I think that's the cleanest approach. The underlying issue is
> > that a bunch of drivers are not making sure that on driver-load they
> > have a consistent sw/hw state for the gamma ramp. As long as you had
> > fbcon enabled the strange ->load_lut call (which isn't really
> > necessary for drivers that got this right) papered over these issues.
> >
> > So a call to you driver's load_lut right affter
> > drm_mode_crtc_set_gamma_size should fix this correctly. a-b:me if you
> > submit that patch.
> 
> Reworked patch attached, commit message changed to summarize the
> Above discussion.
> 
> >
> > /me off for real now
> >
> > Cheers, Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: drm/ast: Linux 4.14 AST DRM driver might not load gamma table [patch proposed]

2017-12-21 Thread Carroll, Lewis
> -Original Message-
> From: daniel.vet...@ffwll.ch [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> Daniel Vetter
> Sent: Thursday, December 21, 2017 3:31 PM
> To: Carroll, Lewis <lewis.carr...@amd.com>; Daenzer, Michel
> <michel.daen...@amd.com>
> Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
> de...@lists.freedesktop.org
> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table
> [patch proposed]
> 
> Fyi Michel, we've discussed similar issues for radeon/amdgpu on irc.
> 
> On Thu, Dec 21, 2017 at 4:21 PM, Carroll, Lewis <lewis.carr...@amd.com>
> wrote:
> >> -Original Message-
> >> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> >> Vetter
> >> Sent: Thursday, December 21, 2017 5:07 AM
> >> To: Carroll, Lewis <lewis.carr...@amd.com>
> >> Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
> >> de...@lists.freedesktop.org
> >> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma
> table
> >> [patch proposed]
> >>
> >> On Thu, Dec 21, 2017 at 12:00:39AM +, Carroll, Lewis wrote:
> >> > The discussion sounds similar as well - related to load_lut() not being
> called.
> >> >
> >> > Perhaps after the drm-next-4.14 merge, whatever call stack used to
> cause
> >> > load_lut to always get called is now gone. Even if
> FB_VISUAL_TRUECOLOR
> >> > doesn't support a clut, some hardware may still need a default gamma
> lut
> >> > loaded (appears to be the case with the AST2500). Perhaps checking for
> >> > that profile and loading the default LUT prepared by
> >> > drm_mode_crtc_set_gamma_size when FB_VISUAL_TRUECOLOR...?
> >>
> >> Yeah drivers should load that somehow. Unfortunately I'm going on
> >> vacations now, so I'm not going to fix anything anytime soon. Might be
> >> good to ping Peter Rosin, he did all the fbdev emulation gamma handling
> >> rework (which is the patch that removed the superflously-looking call to
> >> ->load_lut that some drivers relied on to have a consistent lut state).
> >> -Daniel
> >
> > Enjoy the holidays.
> >
> > Wonder if it would be better to just call load_lut after
> > drm_mode_crtc_set_gamma_size instead of adding a potentially
> > unnecessary DPMS ON event to the commit call-back as I did...? Or call
> > load_lut in the commit callback instead of the DPMS ON call...?
> >
> > The first approach (calling load_lut after set_gamma_size) also works on
> > two test systems.
> 
> Yeah, I think that's the cleanest approach. The underlying issue is
> that a bunch of drivers are not making sure that on driver-load they
> have a consistent sw/hw state for the gamma ramp. As long as you had
> fbcon enabled the strange ->load_lut call (which isn't really
> necessary for drivers that got this right) papered over these issues.
> 
> So a call to you driver's load_lut right affter
> drm_mode_crtc_set_gamma_size should fix this correctly. a-b:me if you
> submit that patch.

Reworked patch attached, commit message changed to summarize the
Above discussion.

> 
> /me off for real now
> 
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch


drm_ast_load_default_gamma_ramp.patch
Description: drm_ast_load_default_gamma_ramp.patch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table [patch proposed]

2017-12-21 Thread Daniel Vetter
Fyi Michel, we've discussed similar issues for radeon/amdgpu on irc.

On Thu, Dec 21, 2017 at 4:21 PM, Carroll, Lewis <lewis.carr...@amd.com> wrote:
>> -Original Message-
>> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
>> Vetter
>> Sent: Thursday, December 21, 2017 5:07 AM
>> To: Carroll, Lewis <lewis.carr...@amd.com>
>> Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
>> de...@lists.freedesktop.org
>> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table
>> [patch proposed]
>>
>> On Thu, Dec 21, 2017 at 12:00:39AM +, Carroll, Lewis wrote:
>> > The discussion sounds similar as well - related to load_lut() not being 
>> > called.
>> >
>> > Perhaps after the drm-next-4.14 merge, whatever call stack used to cause
>> > load_lut to always get called is now gone. Even if FB_VISUAL_TRUECOLOR
>> > doesn't support a clut, some hardware may still need a default gamma lut
>> > loaded (appears to be the case with the AST2500). Perhaps checking for
>> > that profile and loading the default LUT prepared by
>> > drm_mode_crtc_set_gamma_size when FB_VISUAL_TRUECOLOR...?
>>
>> Yeah drivers should load that somehow. Unfortunately I'm going on
>> vacations now, so I'm not going to fix anything anytime soon. Might be
>> good to ping Peter Rosin, he did all the fbdev emulation gamma handling
>> rework (which is the patch that removed the superflously-looking call to
>> ->load_lut that some drivers relied on to have a consistent lut state).
>> -Daniel
>
> Enjoy the holidays.
>
> Wonder if it would be better to just call load_lut after
> drm_mode_crtc_set_gamma_size instead of adding a potentially
> unnecessary DPMS ON event to the commit call-back as I did...? Or call
> load_lut in the commit callback instead of the DPMS ON call...?
>
> The first approach (calling load_lut after set_gamma_size) also works on
> two test systems.

Yeah, I think that's the cleanest approach. The underlying issue is
that a bunch of drivers are not making sure that on driver-load they
have a consistent sw/hw state for the gamma ramp. As long as you had
fbcon enabled the strange ->load_lut call (which isn't really
necessary for drivers that got this right) papered over these issues.

So a call to you driver's load_lut right affter
drm_mode_crtc_set_gamma_size should fix this correctly. a-b:me if you
submit that patch.

/me off for real now

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


RE: drm/ast: Linux 4.14 AST DRM driver might not load gamma table [patch proposed]

2017-12-21 Thread Carroll, Lewis
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Thursday, December 21, 2017 5:07 AM
> To: Carroll, Lewis <lewis.carr...@amd.com>
> Cc: Wentland, Harry <harry.wentl...@amd.com>; dri-
> de...@lists.freedesktop.org
> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table
> [patch proposed]
> 
> On Thu, Dec 21, 2017 at 12:00:39AM +, Carroll, Lewis wrote:
> > The discussion sounds similar as well - related to load_lut() not being 
> > called.
> >
> > Perhaps after the drm-next-4.14 merge, whatever call stack used to cause
> > load_lut to always get called is now gone. Even if FB_VISUAL_TRUECOLOR
> > doesn't support a clut, some hardware may still need a default gamma lut
> > loaded (appears to be the case with the AST2500). Perhaps checking for
> > that profile and loading the default LUT prepared by
> > drm_mode_crtc_set_gamma_size when FB_VISUAL_TRUECOLOR...?
> 
> Yeah drivers should load that somehow. Unfortunately I'm going on
> vacations now, so I'm not going to fix anything anytime soon. Might be
> good to ping Peter Rosin, he did all the fbdev emulation gamma handling
> rework (which is the patch that removed the superflously-looking call to
> ->load_lut that some drivers relied on to have a consistent lut state).
> -Daniel

Enjoy the holidays.

Wonder if it would be better to just call load_lut after
drm_mode_crtc_set_gamma_size instead of adding a potentially
unnecessary DPMS ON event to the commit call-back as I did...? Or call
load_lut in the commit callback instead of the DPMS ON call...?

The first approach (calling load_lut after set_gamma_size) also works on
two test systems.

> 
> >
> > Lewis
> >
> > -Original Message-
> > From: Wentland, Harry
> > Sent: Wednesday, December 20, 2017 6:29 PM
> > To: Carroll, Lewis <lewis.carr...@amd.com>; dri-
> de...@lists.freedesktop.org
> > Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma
> table [patch proposed]
> >
> > This looks similar to this bug that I spotted while finally going through my
> large dri-devel backlog: https://bugzilla.kernel.org/show_bug.cgi?id=198123
> >
> > The discussion continues here https://lists.freedesktop.org/archives/dri-
> devel/2017-December/160667.html
> >
> > Not sure if this is related but the symptoms sound quite similar.
> >
> > Harry
> >
> > On 2017-12-17 07:08 PM, Carroll, Lewis wrote:
> > > Happy Holidays.
> > >
> > >
> > >
> > > Test-driving Linux 4.14 from the Ubuntu Bionic repo on an AMD EPYC
> server using the A-Speed AST2500 BMC and the AST DRM driver and
> framebuffer console configured into the kernel, I have observed multiple
> systems where the analog VGA color palette for the framebuffer console is
> incorrect. Background is blue and all text colors other than white / gray are
> incorrect.
> > >
> > >
> > >
> > > Instrumenting the AST DRM driver, it seems the default gamma table isn't
> loaded to the silicon until there is a DPMS action. When there is a DPMS
> action other than DPMS OFF, the gamma table is loaded and all is well. On
> these boards, the kernel is not calling for a DPMS event at driver init so the
> gamma table is not loaded.
> > >
> > >
> > >
> > > The attached proposed patch adds a DPMS ON event to the commit
> callback after a kernel modeset call. This solves the problem, however there
> may be a better / more proper way to solve this.
> > >
> > >
> > >
> > > Assuming no one on the mailing list has hardware on which to verify this, 
> > > I
> am happy to be a test driver for any suggested fixes.
> > >
> > >
> > >
> > > Regards,
> > >
> > >
> > >
> > > Lewis Carroll
> > >
> > > Principal Solutions Architect
> > >
> > > AMD Enterprise Business Group
> > >
> > >
> > >
> > >
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table [patch proposed]

2017-12-21 Thread Daniel Vetter
On Thu, Dec 21, 2017 at 12:00:39AM +, Carroll, Lewis wrote:
> The discussion sounds similar as well - related to load_lut() not being 
> called.
> 
> Perhaps after the drm-next-4.14 merge, whatever call stack used to cause
> load_lut to always get called is now gone. Even if FB_VISUAL_TRUECOLOR
> doesn't support a clut, some hardware may still need a default gamma lut
> loaded (appears to be the case with the AST2500). Perhaps checking for
> that profile and loading the default LUT prepared by
> drm_mode_crtc_set_gamma_size when FB_VISUAL_TRUECOLOR...?

Yeah drivers should load that somehow. Unfortunately I'm going on
vacations now, so I'm not going to fix anything anytime soon. Might be
good to ping Peter Rosin, he did all the fbdev emulation gamma handling
rework (which is the patch that removed the superflously-looking call to
->load_lut that some drivers relied on to have a consistent lut state).
-Daniel

> 
> Lewis
> 
> -Original Message-
> From: Wentland, Harry 
> Sent: Wednesday, December 20, 2017 6:29 PM
> To: Carroll, Lewis <lewis.carr...@amd.com>; dri-devel@lists.freedesktop.org
> Subject: Re: drm/ast: Linux 4.14 AST DRM driver might not load gamma table 
> [patch proposed]
> 
> This looks similar to this bug that I spotted while finally going through my 
> large dri-devel backlog: https://bugzilla.kernel.org/show_bug.cgi?id=198123
> 
> The discussion continues here 
> https://lists.freedesktop.org/archives/dri-devel/2017-December/160667.html
> 
> Not sure if this is related but the symptoms sound quite similar.
> 
> Harry
> 
> On 2017-12-17 07:08 PM, Carroll, Lewis wrote:
> > Happy Holidays.
> > 
> >  
> > 
> > Test-driving Linux 4.14 from the Ubuntu Bionic repo on an AMD EPYC server 
> > using the A-Speed AST2500 BMC and the AST DRM driver and framebuffer 
> > console configured into the kernel, I have observed multiple systems where 
> > the analog VGA color palette for the framebuffer console is incorrect. 
> > Background is blue and all text colors other than white / gray are 
> > incorrect.
> > 
> >  
> > 
> > Instrumenting the AST DRM driver, it seems the default gamma table isn't 
> > loaded to the silicon until there is a DPMS action. When there is a DPMS 
> > action other than DPMS OFF, the gamma table is loaded and all is well. On 
> > these boards, the kernel is not calling for a DPMS event at driver init so 
> > the gamma table is not loaded.
> > 
> >  
> > 
> > The attached proposed patch adds a DPMS ON event to the commit callback 
> > after a kernel modeset call. This solves the problem, however there may be 
> > a better / more proper way to solve this.
> > 
> >  
> > 
> > Assuming no one on the mailing list has hardware on which to verify this, I 
> > am happy to be a test driver for any suggested fixes.
> > 
> >  
> > 
> > Regards,
> > 
> >  
> > 
> > Lewis Carroll
> > 
> > Principal Solutions Architect
> > 
> > AMD Enterprise Business Group
> > 
> >  
> > 
> > 
> > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel