[PATCHv3 02/30] drm/omap: refactor CRTC HW property setup

[Tomi Valkeinen]

On 29/03/17 11:05, Laurent Pinchart wrote:

>> +static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
>> +{
>> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>> +	struct omap_overlay_manager_info info;
>> +
>> +	memset(&info, 0, sizeof(info));
>> +
>> +	info.default_color = 0x000000;
>> +	info.trans_enabled = false;
>> +	info.partial_alpha_enabled = false;
>> +	info.cpr_enable = false;
> 
> Nitpicking, you could initialize info when declaring it, I think it would be 
> slightly more efficient.

These are open coded here so that it's easy to change these line by line
when we add the properties, and that code most likely can't be in the
initializer. But yes, at the moment it looks a bit silly.

>> +
>> +	dispc_mgr_setup(omap_crtc->channel, &info);
> 
> dispc_mgr_setup() was previously called from omap_crtc_dss_enable() with the 
> DSS disabled. Now it can be called with the DSS enabled. Have you double-
> checked that this isn't an issue ?

It's not an issue, it's how it's supposed to work. These mgr-settings
are "shadow" settings, i.e. they take effect when we set the GO bit
(like the plane settings).

They were only written in dss_enable() as we didn't have support to
change those properties. I hope will get those properties some time
soon, so we need to write the mgr-settings even when the output is enabled.

 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/20170329/50220d95/attachment.sig>