Re: [Intel-gfx] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage

[Daniel Vetter]

On Thu, Jun 22, 2017 at 11:49:34AM +0000, Philippe CORNU wrote:
> 
> 
> On 06/22/2017 08:06 AM, Peter Rosin wrote:
> > The redundant fb helper .load_lut is no longer used, and can not
> > work right without also providing the fb helpers .gamma_set and
> > .gamma_get thus rendering the code in this driver suspect.
> > 
> 
> Hi Peter,
> STM32 chipsets supports 8-bit CLUT mode but this driver version does not 
> support it "yet" (final patch has not been upstreamed because it was a 
> too big fbdev patch for simply adding CLUT...).
> 
> Regarding your patch below, if it helps you to ease the drm framework 
> update then I am agree to "acknowledge it" asap, else if you are not in 
> a hurry, I would prefer a better and definitive patch handling 8-bit 
> CLUT properly and I am ok to help or/and to do it : )

I'll take your ack, since your 8bit lut support will be massively simpler
with Peter's series here :-)

> Extra questions:
> - any plan to update modetest with the DRM_FORMAT_C8 support + gamma 
> get/set?

We have gamma igts, well for the fancy new atomic color manager stuff.
Testing drivers with modetest is kinda not that awesome :-)
-Daniel

> - do you have a simple way to test clut with fbdev, last year we where 
> using an old version of the SDL but I am still looking for a small piece 
> of code to do it (else I will do it myself but C8 on fbdev is not really 
> a priority ;-)
> 
> best regards,
> Philippe
> 
> > Just remove the dead code.
> > 
> > Signed-off-by: Peter Rosin <peda@xxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/stm/ltdc.c | 12 ------------
> >   drivers/gpu/drm/stm/ltdc.h |  1 -
> >   2 files changed, 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > index 1b9483d..87829b9 100644
> > --- a/drivers/gpu/drm/stm/ltdc.c
> > +++ b/drivers/gpu/drm/stm/ltdc.c
> > @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
> >    * DRM_CRTC
> >    */
> >   
> > -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> > -{
> > -	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > -	unsigned int i, lay;
> > -
> > -	for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> > -		for (i = 0; i < 256; i++)
> > -			reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> > -				  ldev->clut[i]);
> > -}
> > -
> >   static void ltdc_crtc_enable(struct drm_crtc *crtc)
> >   {
> >   	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
> >   }
> >   
> >   static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> > -	.load_lut = ltdc_crtc_load_lut,
> >   	.enable = ltdc_crtc_enable,
> >   	.disable = ltdc_crtc_disable,
> >   	.mode_set_nofb = ltdc_crtc_mode_set_nofb,
> > diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> > index d7a9c73..620ca55 100644
> > --- a/drivers/gpu/drm/stm/ltdc.h
> > +++ b/drivers/gpu/drm/stm/ltdc.h
> > @@ -27,7 +27,6 @@ struct ltdc_device {
> >   	struct drm_panel *panel;
> >   	struct mutex err_lock;	/* protecting error_status */
> >   	struct ltdc_caps caps;
> > -	u32 clut[256];		/* color look up table */
> >   	u32 error_status;
> >   	u32 irq_status;
> >   };
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch