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

[Meng Yi <meng.yi@nxp.com>]

> 
> > 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@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;
> 
> Looks good to me, I feel splitting up the register map makes sense anyway. It is
> also documented that way:
> 
> 36.4 Memory Map
> Table 36-5. Memory map
> 
> Parameter                        Address Range
> Register address space           0x0000 – 0x1FFF
> Palette/Tile address space       0x2000 – 0x3FFF
> Gamma_R address space            0x4000 – 0x43FF
> Gamma_G address space            0x4400 – 0x47FF
> Gamma_B address space            0x4800 – 0x4BFF
> Cursor address space             0x4C00 – 0x4FFF

Thanks!^_^

> 
> Can I have an Ack from the device tree maintainers here?
> 
> <snip>
> 
> > --- 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)
> {
> 
> <snip>
> 
> > --- 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,
> 
> I would like to have a comment here why we force little endian.
> 

Oh, I was going to do that, but forgotten anyway.

> > +
> > +	.volatile_reg = fsl_dcu_drm_is_volatile_reg, };
> > +
> 
> <snip>
> 
> > @@ -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);
> > +	}
> > +
> 
> Mark, what do you think, is this a reasonable approach to work around this
> errata?

I was thinking is it possible to define a different endianness for each register map.

Best Regards,
Meng
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel