[RFC/WIP] drm/rockchip: Support CRTC gamma LUT

[Ezequiel Garcia]

On Fri, 2019-06-14 at 13:05 -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jun 13, 2019 at 12:23 PM Ezequiel Garcia <ezequiel at collabora.com> wrote:
> > @@ -1744,6 +1793,41 @@ int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
> >  }
> >  EXPORT_SYMBOL(rockchip_drm_wait_vact_end);
> > 
> > +static int vop_gamma_lut_request(struct device *dev,
> > +                                struct resource *res, struct vop *vop)
> > +{
> > +       resource_size_t offset = vop->data->gamma_lut_addr_off;
> > +       resource_size_t size = VOP_GAMMA_LUT_SIZE * 4;
> > +
> > +       /*
> > +        * Some SoCs (e.g. RK3288) have the gamma LUT address after
> > +        * the MMU registers, which means we can't request and ioremap
> > +        * the entire register set. Other (e.g. RK3399) have gamma LUT
> > +        * address before MMU.
> > +        *
> > +        * Therefore, we need to request and ioremap those that haven't
> > +        * been already.
> > +        */
> > +       if (vop->len >= (offset + size)) {
> > +               vop->lut_regs = vop->regs + offset;
> > +               return 0;
> > +       }
> > +
> > +       if (!devm_request_mem_region(dev, res->start + offset,
> > +                                    size, dev_name(dev))) {
> > +               dev_warn(dev, "can't request gamma lut region\n");
> > +               return -EBUSY;
> > +       }
> > +
> > +       vop->lut_regs = devm_ioremap(dev, res->start + offset, size);
> > +       if (!vop->lut_regs) {
> > +               dev_err(dev, "can't ioremap gamma lut address\n");
> > +               devm_release_mem_region(dev, res->start + offset, size);
> > +               return -ENOMEM;
> > +       }
> 
> I'm curious here.  I was always under the impression that you were
> supposed to specify all of your memory regions in the device tree.
> ...but here the device tree on rk3288 says:
> 
> vopb: vop at ff930000 {
>     compatible = "rockchip,rk3288-vop";
>     reg = <0x0 0xff930000 0x0 0x19c>;
>     ...
> };
> 
> ...and we're now mapping 4096 bytes starting at 0xff931000.  Is that
> really legit?  Wouldn't it be better to put this extra memory range in
> the dts?
> 
> Hrm, but then I guess you need to figure out what to do about older
> device trees.  Do you disable the gamma LUT feature?  ...or do you do
> exactly what the code here is doing and just map it anyway?  I guess
> you could just keep the code here (and it'll work fine), but maybe in
> parallel we should add it to the .dts file and bindings?
> 

Maybe we can see how it would look adding the LUT as a separate
(optional) resource in the devicetree, and dropping support for RK3399,
which doesn't seem to work.

I'm taking a look at Jacopo's question on atomic_flush vs.
atomic_commit_tail, and will prepare a v2.

> ---
> 
> I will say that, though I don't know much (anything?) about gamma
> LUTs, I ran the Chrome OS "gamma_test" program and saw a pretty RGB
> gradient on the both the internal screen and HDMI monitor on my
> rk3288-veyron-jerry.  Thus:
> 
> Tested-by: Douglas Anderson <dianders at chromium.org>

Thanks,
Ezequiel