diff mbox

[v3,3/9] drm/connector: Make ->atomic_commit() optional

Message ID 20180702122020.5921-4-boris.brezillon@bootlin.com (mailing list archive)
State New, archived
Headers show

Commit Message

Boris Brezillon July 2, 2018, 12:20 p.m. UTC 
Not all writeback connector implementations might want to commit things
from the connector driver. Some, like the malidp driver, commit things
from their main commit_tail() function, and would rather not have to
implement a dummy hook for drm_connector_helper_funcs.atomic_commit().

Make this function optional and reflect this fact in the doc.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- New patch
---
 drivers/gpu/drm/drm_atomic_helper.c      | 2 ++
 include/drm/drm_modeset_helper_vtables.h | 2 ++
 2 files changed, 4 insertions(+)

Comments

Liviu Dudau July 2, 2018, 12:56 p.m. UTC | #1 
On Mon, Jul 02, 2018 at 02:20:14PM +0200, Boris Brezillon wrote:
> Not all writeback connector implementations might want to commit things
> from the connector driver. Some, like the malidp driver, commit things
> from their main commit_tail() function, and would rather not have to
> implement a dummy hook for drm_connector_helper_funcs.atomic_commit().
> 
> Make this function optional and reflect this fact in the doc.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks!
Liviu

> ---
> Changes in v3:
> - New patch
> ---
>  drivers/gpu/drm/drm_atomic_helper.c      | 2 ++
>  include/drm/drm_modeset_helper_vtables.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 69063bcf2334..ea19fcc252dc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1184,6 +1184,8 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
>  		const struct drm_connector_helper_funcs *funcs;
>  
>  		funcs = connector->helper_private;
> +		if (!funcs->funcs->atomic_commit)
> +			continue;
>  
>  		if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
>  			WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index fb841f44949c..d0eb76c4b309 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -983,6 +983,8 @@ struct drm_connector_helper_funcs {
>  	 * The writeback_job to commit is available in
>  	 * &drm_connector_state.writeback_job.
>  	 *
> +	 * This hook is optional.
> +	 *
>  	 * This callback is used by the atomic modeset helpers.
>  	 */
>  	void (*atomic_commit)(struct drm_connector *connector,
> -- 
> 2.14.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
kernel test robot July 2, 2018, 3:30 p.m. UTC | #2 
Hi Boris,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20180702]
[cannot apply to drm/drm-next anholt/for-next robh/for-next v4.18-rc3 v4.18-rc2 v4.18-rc1 v4.18-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-vc4-Add-support-for-the-transposer-IP/20180702-211805
config: x86_64-randconfig-x010-201826 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/agp_backend.h:33,
                    from include/drm/drmP.h:35,
                    from drivers/gpu/drm/drm_atomic_helper.c:28:
   drivers/gpu/drm/drm_atomic_helper.c: In function 'drm_atomic_helper_commit_writebacks':
   drivers/gpu/drm/drm_atomic_helper.c:1187:13: error: 'const struct drm_connector_helper_funcs' has no member named 'funcs'
      if (!funcs->funcs->atomic_commit)
                ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/drm_atomic_helper.c:1187:3: note: in expansion of macro 'if'
      if (!funcs->funcs->atomic_commit)
      ^~
   drivers/gpu/drm/drm_atomic_helper.c:1187:13: error: 'const struct drm_connector_helper_funcs' has no member named 'funcs'
      if (!funcs->funcs->atomic_commit)
                ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/gpu/drm/drm_atomic_helper.c:1187:3: note: in expansion of macro 'if'
      if (!funcs->funcs->atomic_commit)
      ^~
   drivers/gpu/drm/drm_atomic_helper.c:1187:13: error: 'const struct drm_connector_helper_funcs' has no member named 'funcs'
      if (!funcs->funcs->atomic_commit)
                ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/gpu/drm/drm_atomic_helper.c:1187:3: note: in expansion of macro 'if'
      if (!funcs->funcs->atomic_commit)
      ^~

vim +/if +1187 drivers/gpu/drm/drm_atomic_helper.c

  1175	
  1176	static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
  1177							struct drm_atomic_state *old_state)
  1178	{
  1179		struct drm_connector *connector;
  1180		struct drm_connector_state *new_conn_state;
  1181		int i;
  1182	
  1183		for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
  1184			const struct drm_connector_helper_funcs *funcs;
  1185	
  1186			funcs = connector->helper_private;
> 1187			if (!funcs->funcs->atomic_commit)
  1188				continue;
  1189	
  1190			if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
  1191				WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
  1192				funcs->atomic_commit(connector, new_conn_state);
  1193			}
  1194		}
  1195	}
  1196	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot July 2, 2018, 3:30 p.m. UTC | #3 
Hi Boris,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20180702]
[cannot apply to drm/drm-next anholt/for-next robh/for-next v4.18-rc3 v4.18-rc2 v4.18-rc1 v4.18-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-vc4-Add-support-for-the-transposer-IP/20180702-211805
config: x86_64-randconfig-x015-201826 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/drm_atomic_helper.c: In function 'drm_atomic_helper_commit_writebacks':
>> drivers/gpu//drm/drm_atomic_helper.c:1187:13: error: 'const struct drm_connector_helper_funcs' has no member named 'funcs'
      if (!funcs->funcs->atomic_commit)
                ^~

vim +1187 drivers/gpu//drm/drm_atomic_helper.c

  1175	
  1176	static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
  1177							struct drm_atomic_state *old_state)
  1178	{
  1179		struct drm_connector *connector;
  1180		struct drm_connector_state *new_conn_state;
  1181		int i;
  1182	
  1183		for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
  1184			const struct drm_connector_helper_funcs *funcs;
  1185	
  1186			funcs = connector->helper_private;
> 1187			if (!funcs->funcs->atomic_commit)
  1188				continue;
  1189	
  1190			if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
  1191				WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
  1192				funcs->atomic_commit(connector, new_conn_state);
  1193			}
  1194		}
  1195	}
  1196	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Boris Brezillon July 2, 2018, 4:41 p.m. UTC | #4 
On Mon, 2 Jul 2018 23:30:05 +0800
kbuild test robot <lkp@intel.com> wrote:

> Hi Boris,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on next-20180702]
> [cannot apply to drm/drm-next anholt/for-next robh/for-next v4.18-rc3 v4.18-rc2 v4.18-rc1 v4.18-rc3]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-vc4-Add-support-for-the-transposer-IP/20180702-211805
> config: x86_64-randconfig-x015-201826 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/gpu//drm/drm_atomic_helper.c: In function 'drm_atomic_helper_commit_writebacks':
> >> drivers/gpu//drm/drm_atomic_helper.c:1187:13: error: 'const struct drm_connector_helper_funcs' has no member named 'funcs'  
>       if (!funcs->funcs->atomic_commit)
>                 ^~
> 
> vim +1187 drivers/gpu//drm/drm_atomic_helper.c
> 
>   1175	
>   1176	static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
>   1177							struct drm_atomic_state *old_state)
>   1178	{
>   1179		struct drm_connector *connector;
>   1180		struct drm_connector_state *new_conn_state;
>   1181		int i;
>   1182	
>   1183		for_each_new_connector_in_state(old_state, connector, new_conn_state, i) {
>   1184			const struct drm_connector_helper_funcs *funcs;
>   1185	
>   1186			funcs = connector->helper_private;
> > 1187			if (!funcs->funcs->atomic_commit)

Oh, shame on me! :-)

I'll send a v4 with this build failure fixed, and this time I'll
compile-test at least. :-)

>   1188				continue;
>   1189	
>   1190			if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
>   1191				WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
>   1192				funcs->atomic_commit(connector, new_conn_state);
>   1193			}
>   1194		}
>   1195	}
>   1196	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 69063bcf2334..ea19fcc252dc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1184,6 +1184,8 @@  static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
 		const struct drm_connector_helper_funcs *funcs;
 
 		funcs = connector->helper_private;
+		if (!funcs->funcs->atomic_commit)
+			continue;
 
 		if (new_conn_state->writeback_job && new_conn_state->writeback_job->fb) {
 			WARN_ON(connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index fb841f44949c..d0eb76c4b309 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -983,6 +983,8 @@  struct drm_connector_helper_funcs {
 	 * The writeback_job to commit is available in
 	 * &drm_connector_state.writeback_job.
 	 *
+	 * This hook is optional.
+	 *
 	 * This callback is used by the atomic modeset helpers.
 	 */
 	void (*atomic_commit)(struct drm_connector *connector,