On 17/05/16 18:36, Daniel Vetter wrote: > On Tue, May 17, 2016 at 7:29 PM, Jon Hunter <jonathanh@xxxxxxxxxx> wrote: >>>> @@ -764,6 +769,9 @@ tegra_dsi_connector_duplicate_state(struct drm_connector *connector) >>>> if (!copy) >>>> return NULL; >>>> >>>> + if (copy->base.crtc) >>>> + drm_connector_reference(connector); >>>> + >>> >>> Please use __drm_atomic_helper_connector_duplicate_state instead of >>> open-coding it. >> >> Unfortunately, tegra is allocating and duplicating memory for the entire >> tegra_dsi_state structure (of which drm_connector_state is a member) in >> this function and so I was not able to do that. However, may be Thierry >> can comment on whether that is completely necessary and if we can move >> to using __drm_atomic_helper_connector_duplicate_state() instead. > > Check out how other drivers are using this helper - it is explicitly > for the case where you duplicate the entire struct, and it just > initializes the core part from drm. You can then add your own fixup > code afterwards. It also doesn't matter whether you do kmalloc or > kcalloc or kmemdup - it does a memcpy of its own to make sure state > gets copied. I had a look but I don't see anyone using the __drm_atomic_helper_connector_duplicate_state() helper, I only see that drivers are using drm_atomic_helper_connector_duplicate_state() directly. Yes I understand that this helper is doing an explicit copy of the entire drm_connector_state struct and yes I could do something like the following ... static struct drm_connector_state * tegra_dsi_connector_duplicate_state(struct drm_connector *connector) { struct tegra_dsi_state *state = to_dsi_state(connector->state); struct tegra_dsi_state *copy; copy = kmemdup(state, sizeof(*state), GFP_KERNEL); if (!copy) return NULL; __drm_atomic_helper_connector_duplicate_state(connector, ©->base); return ©->base; } ... however, this means that I am copying the drm_connector_state twice and this is what I was trying to avoid. Sorry if I am misunderstanding you here, but I don't see how I can avoid the 2nd copy if I use __drm_atomic_helper_connector_duplicate_state(). Cheers Jon -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html