[PATCH v3] drm/fsl-dcu: Implement gamma_lut atomic crtc properties

[Daniel Vetter]

On Wed, Sep 21, 2016 at 11:10:11AM -0700, Stefan Agner wrote:
> On 2016-09-13 01:49, Meng Yi wrote:
> >> > diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig
> >> > b/drivers/gpu/drm/fsl-dcu/Kconfig index 14a72c4..f9c76b1 100644
> >> > --- a/drivers/gpu/drm/fsl-dcu/Kconfig
> >> > +++ b/drivers/gpu/drm/fsl-dcu/Kconfig
> >> > @@ -11,3 +11,9 @@ config DRM_FSL_DCU
> >> >  	help
> >> >  	  Choose this option if you have an Freescale DCU chipset.
> >> >  	  If M is selected the module will be called fsl-dcu-drm.
> >> > +
> >> > +config DRM_FSL_DCU_GAMMA
> >> > +	bool "Gamma Correction Support for NXP/Freescale DCU"
> >> > +	depends on DRM_FSL_DCU
> >> > +	help
> >> > +	  Enable support for gamma correction.
> >>
> >> What is the reason for making this a configuration option? Are there
> >> implementation without support for the Gamma tables?
> >>
> > When gamma correction is enabled, the color won't display normally since
> > The gamma tables are not filled with correct data. So I give a choice
> > to not using
> > The gamma correction when you don't want to use it.
> > 
> > Should I remove this configuration?
> 
> Yeah making this a compile time configuration seems wrong to me.
> 
> I guess we should fill the table with a reasonable default then. The
> omapdrm driver seems to do something similar.

Yup Kconfig is definitely not the correct way to fix a setup problem in
your driver. Note that even when you enable gamma lut userspace is allowed
to set a NULL table, and in that case you need to either disable the gamma
lut, or fill it with a 1:1 mapping (if you hw doesn't have a disable
flag). At boot-up the table will also be NULL (except when you have
special setup code), and if that results in a black screen your driver is
buggy. Papering over a driver bug with Kconfig is definitely not ok.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch