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 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


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

2017-12-17 Thread Carroll, Lewis
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



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