diff mbox

[v2,7/9] drm: Connector helper function to release atomic state

Message ID 1485301777-3465-8-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan Jan. 24, 2017, 11:49 p.m. UTC
Having a ->atomic_release callback is useful to release shared resources
that get allocated in compute_config().

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 include/drm/drm_modeset_helper_vtables.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Daniel Vetter Jan. 25, 2017, 6:18 a.m. UTC | #1
On Tue, Jan 24, 2017 at 03:49:35PM -0800, Dhinakaran Pandiyan wrote:
> Having a ->atomic_release callback is useful to release shared resources
> that get allocated in compute_config().
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  include/drm/drm_modeset_helper_vtables.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 46f5b34..e41b18a 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -831,6 +831,21 @@ struct drm_connector_helper_funcs {
>  	 */
>  	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,
>  						   struct drm_connector_state *connector_state);
> +
> +	/**
> +	 * @atomic_release:
> +	 *
> +	 * This function is used to release shared resources that were
> +	 * previously acquired. For example, resources acquired in
> +	 * encoder->compute_config() can be released by calling this function

@compute_config is the right way to do references within the same struct.

> +	 * from mode_fixup()

Same here.

Patch split up is a bit strange, hence why my review of the design is in
later patches.

Thanks, Daniel

> +	 *
> +	 * NOTE:
> +	 *
> +	 * This function is called in the check phase of an atomic update.
> +	 */
> +	void (*atomic_release)(struct drm_connector *connector,
> +			       struct drm_connector_state *connector_state);
>  };
>  
>  /**
> -- 
> 2.7.4
>
Dhinakaran Pandiyan Jan. 25, 2017, 9:01 p.m. UTC | #2
On Wed, 2017-01-25 at 07:18 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:35PM -0800, Dhinakaran Pandiyan wrote:

> > Having a ->atomic_release callback is useful to release shared resources

> > that get allocated in compute_config().

> > 

> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  include/drm/drm_modeset_helper_vtables.h | 15 +++++++++++++++

> >  1 file changed, 15 insertions(+)

> > 

> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h

> > index 46f5b34..e41b18a 100644

> > --- a/include/drm/drm_modeset_helper_vtables.h

> > +++ b/include/drm/drm_modeset_helper_vtables.h

> > @@ -831,6 +831,21 @@ struct drm_connector_helper_funcs {

> >  	 */

> >  	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,

> >  						   struct drm_connector_state *connector_state);

> > +

> > +	/**

> > +	 * @atomic_release:

> > +	 *

> > +	 * This function is used to release shared resources that were

> > +	 * previously acquired. For example, resources acquired in

> > +	 * encoder->compute_config() can be released by calling this function

> 

> @compute_config is the right way to do references within the same struct.

> 

> > +	 * from mode_fixup()

> 

> Same here.

> 

> Patch split up is a bit strange, hence why my review of the design is in

> later patches.

> 

> Thanks, Daniel

> 



I'll squash them appropriately, thanks for the review.

-DK
> > +	 *

> > +	 * NOTE:

> > +	 *

> > +	 * This function is called in the check phase of an atomic update.

> > +	 */

> > +	void (*atomic_release)(struct drm_connector *connector,

> > +			       struct drm_connector_state *connector_state);

> >  };

> >  

> >  /**

> > -- 

> > 2.7.4

> > 

>
Dhinakaran Pandiyan Jan. 31, 2017, 12:14 a.m. UTC | #3
On Wed, 2017-01-25 at 07:18 +0100, Daniel Vetter wrote:
> On Tue, Jan 24, 2017 at 03:49:35PM -0800, Dhinakaran Pandiyan wrote:

> > Having a ->atomic_release callback is useful to release shared resources

> > that get allocated in compute_config().

> > 

> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > ---

> >  include/drm/drm_modeset_helper_vtables.h | 15 +++++++++++++++

> >  1 file changed, 15 insertions(+)

> > 

> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h

> > index 46f5b34..e41b18a 100644

> > --- a/include/drm/drm_modeset_helper_vtables.h

> > +++ b/include/drm/drm_modeset_helper_vtables.h

> > @@ -831,6 +831,21 @@ struct drm_connector_helper_funcs {

> >  	 */

> >  	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,

> >  						   struct drm_connector_state *connector_state);

> > +

> > +	/**

> > +	 * @atomic_release:

> > +	 *

> > +	 * This function is used to release shared resources that were

> > +	 * previously acquired. For example, resources acquired in

> > +	 * encoder->compute_config() can be released by calling this function

> 

> @compute_config is the right way to do references within the same struct.


compute_config is not in the same structure, which made me realize I
should not be referring to compute_config() at all, as it is a helper
for struct intel_encoder. 


-DK
> 

> > +	 * from mode_fixup()

> 

> Same here.

> 

> Patch split up is a bit strange, hence why my review of the design is in

> later patches.

> 

> Thanks, Daniel

> 

> > +	 *

> > +	 * NOTE:

> > +	 *

> > +	 * This function is called in the check phase of an atomic update.

> > +	 */

> > +	void (*atomic_release)(struct drm_connector *connector,

> > +			       struct drm_connector_state *connector_state);

> >  };

> >  

> >  /**

> > -- 

> > 2.7.4

> > 

>
diff mbox

Patch

diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 46f5b34..e41b18a 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -831,6 +831,21 @@  struct drm_connector_helper_funcs {
 	 */
 	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,
 						   struct drm_connector_state *connector_state);
+
+	/**
+	 * @atomic_release:
+	 *
+	 * This function is used to release shared resources that were
+	 * previously acquired. For example, resources acquired in
+	 * encoder->compute_config() can be released by calling this function
+	 * from mode_fixup()
+	 *
+	 * NOTE:
+	 *
+	 * This function is called in the check phase of an atomic update.
+	 */
+	void (*atomic_release)(struct drm_connector *connector,
+			       struct drm_connector_state *connector_state);
 };
 
 /**