Hi Meng,

One more thing which I have my concern:

On 2016-09-07 02:22, Meng Yi wrote:
> Gamma correction is optional and can be used to adjust the color
> output values to match the gamut of a particular TFT LCD panel
> Errata:
> Gamma_R, Gamma_G and Gamma_B registers are little-endian registers
> while the rest of the address-space in 2D-ACE is big-endian.
> Workaround:
> Split the DCU regs into "regs", "palette", "gamma" and "cursor".
> Create a second regmap for gamma memory space using little endian.
> 
> Suggested-by: Stefan Agner <stefan at agner.ch>
> Signed-off-by: Meng Yi <meng.yi at nxp.com>
> ---
> Changes in V3:
> -created a second regmap for gamma
> -updated the DCU DT binding
> ---
>  .../devicetree/bindings/display/fsl,dcu.txt        |  6 +++-
>  drivers/gpu/drm/fsl-dcu/Kconfig                    |  6 ++++
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c         | 38 
> ++++++++++++++++++++++
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c          | 30 ++++++++++++++++-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h          |  8 +++++
>  5 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> index 63ec2a6..1b1321a 100644
> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> @@ -20,7 +20,11 @@ Optional properties:
>  Examples:
>  dcu: dcu at 2ce0000 {
>       compatible = "fsl,ls1021a-dcu";
> -     reg = <0x0 0x2ce0000 0x0 0x10000>;
> +     reg = <0x0 0x2ce0000 0x0 0x2000>,
> +           <0x0 0x2ce2000 0x0 0x2000>,
> +           <0x0 0x2ce4000 0x0 0xc00>,
> +           <0x0 0x2ce4c00 0x0 0x400>;
> +     reg-names = "regs", "palette", "gamma", "cursor";
>       clocks = <&platform_clk 0>, <&platform_clk 0>;
>       clock-names = "dcu", "pix";
>       big-endian;
> 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?

--
Stefan

> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> index 3371635..25ab0ff 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> @@ -22,6 +22,22 @@
>  #include "fsl_dcu_drm_drv.h"
>  #include "fsl_dcu_drm_plane.h"
>  
> +static void fsl_crtc_gamma_set(struct drm_crtc *crtc, struct
> drm_color_lut *lut,
> +                           uint32_t size)
> +{
> +     struct fsl_dcu_drm_device *fsl_dev = crtc->dev->dev_private;
> +     unsigned int i;
> +
> +     for (i = 0; i < size; i++) {
> +             regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_R + 4 * i,
> +                          lut[i].red);
> +             regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_G + 4 * i,
> +                          lut[i].green);
> +             regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_B + 4 * i,
> +                          lut[i].blue);
> +     }
> +}
> +
>  static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc,
>                                         struct drm_crtc_state *old_crtc_state)
>  {
> @@ -37,6 +53,11 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct
> drm_crtc *crtc,
>                       drm_crtc_send_vblank_event(crtc, event);
>               spin_unlock_irq(&crtc->dev->event_lock);
>       }
> +
> +     if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut)
> +             fsl_crtc_gamma_set(crtc, (struct drm_color_lut *)
> +                                crtc->state->gamma_lut->data,
> +                                256);
>  }
>  
>  static void fsl_dcu_drm_disable_crtc(struct drm_crtc *crtc)
> @@ -46,6 +67,10 @@ static void fsl_dcu_drm_disable_crtc(struct drm_crtc *crtc)
>  
>       drm_crtc_vblank_off(crtc);
>  
> +     if (fsl_dev->enable_color_mgmt)
> +             regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
> +                                DCU_MODE_EN_GAMMA_MASK, 0);
> +
>       regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
>                          DCU_MODE_DCU_MODE_MASK,
>                          DCU_MODE_DCU_MODE(DCU_MODE_OFF));
> @@ -58,6 +83,11 @@ static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
>       struct drm_device *dev = crtc->dev;
>       struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>  
> +     if (fsl_dev->enable_color_mgmt)
> +             regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
> +                                DCU_MODE_EN_GAMMA_MASK,
> +                                DCU_MODE_GAMMA_ENABLE);
> +
>       regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
>                          DCU_MODE_DCU_MODE_MASK,
>                          DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
> @@ -135,6 +165,7 @@ static const struct drm_crtc_funcs
> fsl_dcu_drm_crtc_funcs = {
>       .page_flip = drm_atomic_helper_page_flip,
>       .reset = drm_atomic_helper_crtc_reset,
>       .set_config = drm_atomic_helper_set_config,
> +     .gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
> @@ -158,5 +189,12 @@ int fsl_dcu_drm_crtc_create(struct
> fsl_dcu_drm_device *fsl_dev)
>  
>       drm_crtc_helper_add(crtc, &fsl_dcu_drm_crtc_helper_funcs);
>  
> +#ifdef CONFIG_DRM_FSL_DCU_GAMMA
> +     fsl_dev->enable_color_mgmt = true;
> +
> +     drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
> +     drm_mode_crtc_set_gamma_size(crtc, 256);
> +#endif
> +
>       return 0;
>  }
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index 7882387..f70ea68 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -48,6 +48,15 @@ static const struct regmap_config fsl_dcu_regmap_config = {
>       .volatile_reg = fsl_dcu_drm_is_volatile_reg,
>  };
>  
> +static const struct regmap_config fsl_dcu_regmap_gamma_config = {
> +     .reg_bits = 32,
> +     .reg_stride = 4,
> +     .val_bits = 32,
> +     .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +
> +     .volatile_reg = fsl_dcu_drm_is_volatile_reg,
> +};
> +
>  static int fsl_dcu_drm_irq_init(struct drm_device *dev)
>  {
>       struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> @@ -323,7 +332,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>       struct drm_device *drm;
>       struct device *dev = &pdev->dev;
>       struct resource *res;
> -     void __iomem *base;
> +     void __iomem *base, *base_gamma;
>       struct drm_driver *driver = &fsl_dcu_drm_driver;
>       struct clk *pix_clk_in;
>       char pix_clk_name[32];
> @@ -365,6 +374,25 @@ static int fsl_dcu_drm_probe(struct platform_device 
> *pdev)
>               return PTR_ERR(fsl_dev->regmap);
>       }
>  
> +     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gamma");
> +     if (!res) {
> +             dev_err(dev, "could not get gamma memory resource\n");
> +             return -ENODEV;
> +     }
> +
> +     base_gamma = devm_ioremap_resource(dev, res);
> +     if (IS_ERR(base_gamma)) {
> +             ret = PTR_ERR(base_gamma);
> +             return ret;
> +     }
> +
> +     fsl_dev->regmap_gamma = devm_regmap_init_mmio(dev, base_gamma,
> +                     &fsl_dcu_regmap_config);
> +     if (IS_ERR(fsl_dev->regmap_gamma)) {
> +             dev_err(dev, "regmap_gamma init failed\n");
> +             return PTR_ERR(fsl_dev->regmap_gamma);
> +     }
> +
>       fsl_dev->clk = devm_clk_get(dev, "dcu");
>       if (IS_ERR(fsl_dev->clk)) {
>               dev_err(dev, "failed to get dcu clock\n");
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> index 3b371fe7..95427f6 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -25,6 +25,8 @@
>  #define DCU_MODE_NORMAL                      1
>  #define DCU_MODE_TEST                        2
>  #define DCU_MODE_COLORBAR            3
> +#define DCU_MODE_EN_GAMMA_MASK               0x04
> +#define DCU_MODE_GAMMA_ENABLE                BIT(2)
>  
>  #define DCU_BGND                     0x0014
>  #define DCU_BGND_R(x)                        ((x) << 16)
> @@ -165,6 +167,10 @@
>  #define VF610_LAYER_REG_NUM          9
>  #define LS1021A_LAYER_REG_NUM                10
>  
> +#define FSL_GAMMA_R                  0x000
> +#define FSL_GAMMA_G                  0x400
> +#define FSL_GAMMA_B                  0x800
> +
>  struct clk;
>  struct device;
>  struct drm_device;
> @@ -182,6 +188,7 @@ struct fsl_dcu_drm_device {
>       struct device *dev;
>       struct device_node *np;
>       struct regmap *regmap;
> +     struct regmap *regmap_gamma;
>       int irq;
>       struct clk *clk;
>       struct clk *pix_clk;
> @@ -195,6 +202,7 @@ struct fsl_dcu_drm_device {
>       struct fsl_dcu_drm_connector connector;
>       const struct fsl_dcu_soc_data *soc;
>       struct drm_atomic_state *state;
> +     bool enable_color_mgmt;
>  };
>  
>  void fsl_dcu_fbdev_init(struct drm_device *dev);

Reply via email to