[PATCH v3 02/11] drm/tilcdc: implement palette loading for rev1

[Tomi Valkeinen]

On 24/11/16 11:39, Jyri Sarha wrote:
> On 11/24/16 11:29, Tomi Valkeinen wrote:
>>> @@ -213,6 +274,13 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
>>>>  				__func__);
>>>>  	}
>>>>  
>>>> +	/*
>>>> +	 * LCDC will not retain the palette when reset. Make sure it gets
>>>> +	 * reloaded on tilcdc_crtc_enable().
>>>> +	 */
>>>> +	if (priv->rev == 1)
>>>> +		reinit_completion(&tilcdc_crtc->palette_loaded);
>>>> +
>> So when the crtc is disabled, you do this, so that on crtc enable the
>> driver would load palette? When is the crtc enabled if it hasn't been
>> disabled first? In other words, when will the palette loading in
>> tilcdc_crtc_enable() _not_ trigger for v1?
>>
>> This looks a bit messy to me.
>>
>> Why not just load the palette every time on crtc enable? And reinit the
>> completion in tilcdc_crtc_load_palette().
>>
> 
> My final version loads it at the end of modeset_nofb(), to avoid this:
> 
> 1. Load the fb address to dma registers with "ingenious" 64 bit write
> 
> 2. Load the dma registers to a temporary storrage
> 
> 3. Put palette address in dma registers and load the palette
> 
> 4. Restore the dma registers (= fb address)
> 
> But, sure. I can load the palette every time the mode changes, but not
> every time the display is enabled.

What is the difference? If mode changes, you need to disable and enable
the crtc, right? What other cases there are to enable the display? After
blank? Then the display has been off, and I presume palette has to be
loaded.

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161124/b5e191b4/attachment.sig>