Re: [Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

2017-01-30 Thread Pandiyan, Dhinakaran
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 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  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
> > 
> 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

2017-01-25 Thread Pandiyan, Dhinakaran
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 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  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
> > 
> 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

2017-01-24 Thread Daniel Vetter
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 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2 7/9] drm: Connector helper function to release atomic state

2017-01-24 Thread Dhinakaran Pandiyan
Having a ->atomic_release callback is useful to release shared resources
that get allocated in compute_config().

Suggested-by: Daniel Vetter 
Signed-off-by: Dhinakaran Pandiyan 
---
 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
+* 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);
 };
 
 /**
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx