All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm: Fix some typo mistake of the annotations
@ 2015-03-17  7:30 John Hunter
  2015-03-17  7:30 ` [PATCH 2/3] drm: replace the 'for' condition with outside defined variable John Hunter
  2015-03-17  7:30 ` [PATCH 3/3] drm: change connector to tmp_connector John Hunter
  0 siblings, 2 replies; 8+ messages in thread
From: John Hunter @ 2015-03-17  7:30 UTC (permalink / raw)
  To: dri-devel

There are some mistakes that the function name in the annotaions
is not matching the real function name.
And some duplication word in annotations

Signed-off-by: John Hunter <zhjwpku@gmail.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a745881..39369ee 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -346,7 +346,7 @@ needs_modeset(struct drm_crtc_state *state)
 }
 
 /**
- * drm_atomic_helper_check - validate state object for modeset changes
+ * drm_atomic_helper_check_modeset - validate state object for modeset changes
  * @dev: DRM device
  * @state: the driver state object
  *
@@ -461,7 +461,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
 
 /**
- * drm_atomic_helper_check - validate state object for modeset changes
+ * drm_atomic_helper_check_planes - validate state object for planes changes
  * @dev: DRM device
  * @state: the driver state object
  *
@@ -605,7 +605,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
-		 * it away), so we won't call call disable hooks twice.
+		 * it away), so we won't call disable hooks twice.
 		 */
 		if (encoder->bridge)
 			encoder->bridge->funcs->disable(encoder->bridge);
@@ -757,7 +757,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
-		 * it away), so we won't call call mode_set hooks twice.
+		 * it away), so we won't call mode_set hooks twice.
 		 */
 		if (funcs->mode_set)
 			funcs->mode_set(encoder, mode, adjusted_mode);
@@ -858,7 +858,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 
 		/*
 		 * Each encoder has at most one connector (since we always steal
-		 * it away), so we won't call call enable hooks twice.
+		 * it away), so we won't call enable hooks twice.
 		 */
 		if (encoder->bridge)
 			encoder->bridge->funcs->pre_enable(encoder->bridge);
@@ -1025,7 +1025,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 
 	/*
 	 * Everything below can be run asynchronously without the need to grab
-	 * any modeset locks at all under one conditions: It must be guaranteed
+	 * any modeset locks at all under one condition: It must be guaranteed
 	 * that the asynchronous work has either been cancelled (if the driver
 	 * supports it, which at least requires that the framebuffers get
 	 * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm: replace the 'for' condition with outside defined variable
  2015-03-17  7:30 [PATCH 1/3] drm: Fix some typo mistake of the annotations John Hunter
@ 2015-03-17  7:30 ` John Hunter
  2015-03-17  8:40   ` Daniel Vetter
  2015-03-17  7:30 ` [PATCH 3/3] drm: change connector to tmp_connector John Hunter
  1 sibling, 1 reply; 8+ messages in thread
From: John Hunter @ 2015-03-17  7:30 UTC (permalink / raw)
  To: dri-devel

use outdise defined variable can reduce the recaculate of the
count of planes, crtcs and connectors.

Signed-off-by: John Hunter <zhjwpku@gmail.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 39369ee..20376e6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1301,8 +1301,11 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
 				  struct drm_atomic_state *state)
 {
 	int i;
+	int nconnectors = dev->mode_config.num_connector;
+	int ncrtcs = dev->mode_config.num_crtc;
+	int nplanes = dev->mode_config.num_total_plane;
 
-	for (i = 0; i < dev->mode_config.num_connector; i++) {
+	for (i = 0; i < nconnectors; i++) {
 		struct drm_connector *connector = state->connectors[i];
 
 		if (!connector)
@@ -1313,7 +1316,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
 		connector->state->state = NULL;
 	}
 
-	for (i = 0; i < dev->mode_config.num_crtc; i++) {
+	for (i = 0; i < ncrtcs; i++) {
 		struct drm_crtc *crtc = state->crtcs[i];
 
 		if (!crtc)
@@ -1324,7 +1327,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
 		crtc->state->state = NULL;
 	}
 
-	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
+	for (i = 0; i < nplanes; i++) {
 		struct drm_plane *plane = state->planes[i];
 
 		if (!plane)
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] drm: change connector to tmp_connector
  2015-03-17  7:30 [PATCH 1/3] drm: Fix some typo mistake of the annotations John Hunter
  2015-03-17  7:30 ` [PATCH 2/3] drm: replace the 'for' condition with outside defined variable John Hunter
@ 2015-03-17  7:30 ` John Hunter
  2015-03-17  8:39   ` Daniel Vetter
  1 sibling, 1 reply; 8+ messages in thread
From: John Hunter @ 2015-03-17  7:30 UTC (permalink / raw)
  To: dri-devel

This wasn't too harmful since we already look at connector,
which has the same effect as the loop for any non-cloned configs.
Only when we have a cloned configuration is it important to look
at other connectors. Furthermore existing userspace always changes
dpms on all of them anyway.

Signed-off-by: JohnHunter <zhjwpku@gmail.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 20376e6..93b467d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2004,10 +2004,10 @@ retry:
 	WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
 
 	list_for_each_entry(tmp_connector, &config->connector_list, head) {
-		if (connector->state->crtc != crtc)
+		if (tmp_connector->state->crtc != crtc)
 			continue;
 
-		if (connector->dpms == DRM_MODE_DPMS_ON) {
+		if (tmp_connector->dpms == DRM_MODE_DPMS_ON) {
 			active = true;
 			break;
 		}
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] drm: change connector to tmp_connector
  2015-03-17  7:30 ` [PATCH 3/3] drm: change connector to tmp_connector John Hunter
@ 2015-03-17  8:39   ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2015-03-17  8:39 UTC (permalink / raw)
  To: John Hunter; +Cc: dri-devel

On Tue, Mar 17, 2015 at 03:30:28PM +0800, John Hunter wrote:
> This wasn't too harmful since we already look at connector,
> which has the same effect as the loop for any non-cloned configs.
> Only when we have a cloned configuration is it important to look
> at other connectors. Furthermore existing userspace always changes
> dpms on all of them anyway.
> 
> Signed-off-by: JohnHunter <zhjwpku@gmail.com>

Another small one about process: When resending patches please have some
per-patch changelog about what you've changed here. E.g. for this one add
a line like:

v2: Update commit message as per discussion with Daniel.

It's also good practice to cc anyone who took part in previous discussions
of a patch. If you add a line like

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

at the bottom of the commit message (right above the sob line) then git
send-email will automatically pick it up. This way people won't miss your
next patch iteration in the flood of mails they tend to get ;-)

Anyway thanks a lot for your patches, I've merged 1&3 from this series to
drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 20376e6..93b467d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2004,10 +2004,10 @@ retry:
>  	WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
>  
>  	list_for_each_entry(tmp_connector, &config->connector_list, head) {
> -		if (connector->state->crtc != crtc)
> +		if (tmp_connector->state->crtc != crtc)
>  			continue;
>  
> -		if (connector->dpms == DRM_MODE_DPMS_ON) {
> +		if (tmp_connector->dpms == DRM_MODE_DPMS_ON) {
>  			active = true;
>  			break;
>  		}
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] drm: replace the 'for' condition with outside defined variable
  2015-03-17  7:30 ` [PATCH 2/3] drm: replace the 'for' condition with outside defined variable John Hunter
@ 2015-03-17  8:40   ` Daniel Vetter
  2015-03-17  8:48     ` John Hunter
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2015-03-17  8:40 UTC (permalink / raw)
  To: John Hunter; +Cc: dri-devel

On Tue, Mar 17, 2015 at 03:30:27PM +0800, John Hunter wrote:
> use outdise defined variable can reduce the recaculate of the
> count of planes, crtcs and connectors.
> 
> Signed-off-by: John Hunter <zhjwpku@gmail.com>

Hm, what's the benefit you see for this change? The lines aren't too long
yet and we don't reuse the expression, so imo code readability isn't
improved.
-Daniel
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 39369ee..20376e6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1301,8 +1301,11 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
>  				  struct drm_atomic_state *state)
>  {
>  	int i;
> +	int nconnectors = dev->mode_config.num_connector;
> +	int ncrtcs = dev->mode_config.num_crtc;
> +	int nplanes = dev->mode_config.num_total_plane;
>  
> -	for (i = 0; i < dev->mode_config.num_connector; i++) {
> +	for (i = 0; i < nconnectors; i++) {
>  		struct drm_connector *connector = state->connectors[i];
>  
>  		if (!connector)
> @@ -1313,7 +1316,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
>  		connector->state->state = NULL;
>  	}
>  
> -	for (i = 0; i < dev->mode_config.num_crtc; i++) {
> +	for (i = 0; i < ncrtcs; i++) {
>  		struct drm_crtc *crtc = state->crtcs[i];
>  
>  		if (!crtc)
> @@ -1324,7 +1327,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
>  		crtc->state->state = NULL;
>  	}
>  
> -	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> +	for (i = 0; i < nplanes; i++) {
>  		struct drm_plane *plane = state->planes[i];
>  
>  		if (!plane)
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] drm: replace the 'for' condition with outside defined variable
  2015-03-17  8:40   ` Daniel Vetter
@ 2015-03-17  8:48     ` John Hunter
  2015-03-17  9:24       ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: John Hunter @ 2015-03-17  8:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2970 bytes --]

Hi Daniel,

On Tue, Mar 17, 2015 at 4:40 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Mar 17, 2015 at 03:30:27PM +0800, John Hunter wrote:
> > use outdise defined variable can reduce the recaculate of the
> > count of planes, crtcs and connectors.
> >
> > Signed-off-by: John Hunter <zhjwpku@gmail.com>
>
> Hm, what's the benefit you see for this change? The lines aren't too long
> yet and we don't reuse the expression, so imo code readability isn't
> improved.
>
I change this just reference some other functions in the same file.
like,
     drm_atomic_helper_check_planes
     wait_for_fences
     ...
I really think we should keep the same coding style in the same file.
If I am wrong with that, just ignore this patch :-)

> -Daniel
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> > index 39369ee..20376e6 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1301,8 +1301,11 @@ void drm_atomic_helper_swap_state(struct
> drm_device *dev,
> >                                 struct drm_atomic_state *state)
> >  {
> >       int i;
> > +     int nconnectors = dev->mode_config.num_connector;
> > +     int ncrtcs = dev->mode_config.num_crtc;
> > +     int nplanes = dev->mode_config.num_total_plane;
> >
> > -     for (i = 0; i < dev->mode_config.num_connector; i++) {
> > +     for (i = 0; i < nconnectors; i++) {
> >               struct drm_connector *connector = state->connectors[i];
> >
> >               if (!connector)
> > @@ -1313,7 +1316,7 @@ void drm_atomic_helper_swap_state(struct
> drm_device *dev,
> >               connector->state->state = NULL;
> >       }
> >
> > -     for (i = 0; i < dev->mode_config.num_crtc; i++) {
> > +     for (i = 0; i < ncrtcs; i++) {
> >               struct drm_crtc *crtc = state->crtcs[i];
> >
> >               if (!crtc)
> > @@ -1324,7 +1327,7 @@ void drm_atomic_helper_swap_state(struct
> drm_device *dev,
> >               crtc->state->state = NULL;
> >       }
> >
> > -     for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> > +     for (i = 0; i < nplanes; i++) {
> >               struct drm_plane *plane = state->planes[i];
> >
> >               if (!plane)
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>



-- 
Best regards
Junwang Zhao
Microprocessor Research and Develop Center
Department of Computer Science &Technology
Peking University
Beijing, 100871, PRC

[-- Attachment #1.2: Type: text/html, Size: 4753 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] drm: replace the 'for' condition with outside defined variable
  2015-03-17  8:48     ` John Hunter
@ 2015-03-17  9:24       ` Daniel Vetter
  2015-03-17  9:29         ` John Hunter
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2015-03-17  9:24 UTC (permalink / raw)
  To: John Hunter; +Cc: dri-devel

On Tue, Mar 17, 2015 at 04:48:23PM +0800, John Hunter wrote:
> Hi Daniel,
> 
> On Tue, Mar 17, 2015 at 4:40 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Tue, Mar 17, 2015 at 03:30:27PM +0800, John Hunter wrote:
> > > use outdise defined variable can reduce the recaculate of the
> > > count of planes, crtcs and connectors.
> > >
> > > Signed-off-by: John Hunter <zhjwpku@gmail.com>
> >
> > Hm, what's the benefit you see for this change? The lines aren't too long
> > yet and we don't reuse the expression, so imo code readability isn't
> > improved.
> >
> I change this just reference some other functions in the same file.
> like,
>      drm_atomic_helper_check_planes
>      wait_for_fences
>      ...
> I really think we should keep the same coding style in the same file.
> If I am wrong with that, just ignore this patch :-)

Indeed that's a bit inconsistent. But in cases like these where neither
approach is really better I usually go with "don't change anything". Btw
for the next patch the above explanation should be in the commit message.
The important part isn't really explaining what you change (the code
should be readable enough to make that clear), but _why_ you change
something.
-Daniel

> 
> > -Daniel
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 39369ee..20376e6 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -1301,8 +1301,11 @@ void drm_atomic_helper_swap_state(struct
> > drm_device *dev,
> > >                                 struct drm_atomic_state *state)
> > >  {
> > >       int i;
> > > +     int nconnectors = dev->mode_config.num_connector;
> > > +     int ncrtcs = dev->mode_config.num_crtc;
> > > +     int nplanes = dev->mode_config.num_total_plane;
> > >
> > > -     for (i = 0; i < dev->mode_config.num_connector; i++) {
> > > +     for (i = 0; i < nconnectors; i++) {
> > >               struct drm_connector *connector = state->connectors[i];
> > >
> > >               if (!connector)
> > > @@ -1313,7 +1316,7 @@ void drm_atomic_helper_swap_state(struct
> > drm_device *dev,
> > >               connector->state->state = NULL;
> > >       }
> > >
> > > -     for (i = 0; i < dev->mode_config.num_crtc; i++) {
> > > +     for (i = 0; i < ncrtcs; i++) {
> > >               struct drm_crtc *crtc = state->crtcs[i];
> > >
> > >               if (!crtc)
> > > @@ -1324,7 +1327,7 @@ void drm_atomic_helper_swap_state(struct
> > drm_device *dev,
> > >               crtc->state->state = NULL;
> > >       }
> > >
> > > -     for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> > > +     for (i = 0; i < nplanes; i++) {
> > >               struct drm_plane *plane = state->planes[i];
> > >
> > >               if (!plane)
> > > --
> > > 1.9.1
> > >
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> 
> 
> 
> -- 
> Best regards
> Junwang Zhao
> Microprocessor Research and Develop Center
> Department of Computer Science &Technology
> Peking University
> Beijing, 100871, PRC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] drm: replace the 'for' condition with outside defined variable
  2015-03-17  9:24       ` Daniel Vetter
@ 2015-03-17  9:29         ` John Hunter
  0 siblings, 0 replies; 8+ messages in thread
From: John Hunter @ 2015-03-17  9:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 4138 bytes --]

Got it!

Cheers,
John

On Tue, Mar 17, 2015 at 5:24 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Mar 17, 2015 at 04:48:23PM +0800, John Hunter wrote:
> > Hi Daniel,
> >
> > On Tue, Mar 17, 2015 at 4:40 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > > On Tue, Mar 17, 2015 at 03:30:27PM +0800, John Hunter wrote:
> > > > use outdise defined variable can reduce the recaculate of the
> > > > count of planes, crtcs and connectors.
> > > >
> > > > Signed-off-by: John Hunter <zhjwpku@gmail.com>
> > >
> > > Hm, what's the benefit you see for this change? The lines aren't too
> long
> > > yet and we don't reuse the expression, so imo code readability isn't
> > > improved.
> > >
> > I change this just reference some other functions in the same file.
> > like,
> >      drm_atomic_helper_check_planes
> >      wait_for_fences
> >      ...
> > I really think we should keep the same coding style in the same file.
> > If I am wrong with that, just ignore this patch :-)
>
> Indeed that's a bit inconsistent. But in cases like these where neither
> approach is really better I usually go with "don't change anything". Btw
> for the next patch the above explanation should be in the commit message.
> The important part isn't really explaining what you change (the code
> should be readable enough to make that clear), but _why_ you change
> something.
> -Daniel
>
> >
> > > -Daniel
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index 39369ee..20376e6 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -1301,8 +1301,11 @@ void drm_atomic_helper_swap_state(struct
> > > drm_device *dev,
> > > >                                 struct drm_atomic_state *state)
> > > >  {
> > > >       int i;
> > > > +     int nconnectors = dev->mode_config.num_connector;
> > > > +     int ncrtcs = dev->mode_config.num_crtc;
> > > > +     int nplanes = dev->mode_config.num_total_plane;
> > > >
> > > > -     for (i = 0; i < dev->mode_config.num_connector; i++) {
> > > > +     for (i = 0; i < nconnectors; i++) {
> > > >               struct drm_connector *connector = state->connectors[i];
> > > >
> > > >               if (!connector)
> > > > @@ -1313,7 +1316,7 @@ void drm_atomic_helper_swap_state(struct
> > > drm_device *dev,
> > > >               connector->state->state = NULL;
> > > >       }
> > > >
> > > > -     for (i = 0; i < dev->mode_config.num_crtc; i++) {
> > > > +     for (i = 0; i < ncrtcs; i++) {
> > > >               struct drm_crtc *crtc = state->crtcs[i];
> > > >
> > > >               if (!crtc)
> > > > @@ -1324,7 +1327,7 @@ void drm_atomic_helper_swap_state(struct
> > > drm_device *dev,
> > > >               crtc->state->state = NULL;
> > > >       }
> > > >
> > > > -     for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> > > > +     for (i = 0; i < nplanes; i++) {
> > > >               struct drm_plane *plane = state->planes[i];
> > > >
> > > >               if (!plane)
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> >
> >
> >
> > --
> > Best regards
> > Junwang Zhao
> > Microprocessor Research and Develop Center
> > Department of Computer Science &Technology
> > Peking University
> > Beijing, 100871, PRC
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>



-- 
Best regards
Junwang Zhao
Microprocessor Research and Develop Center
Department of Computer Science &Technology
Peking University
Beijing, 100871, PRC

[-- Attachment #1.2: Type: text/html, Size: 6422 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-03-17  9:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-17  7:30 [PATCH 1/3] drm: Fix some typo mistake of the annotations John Hunter
2015-03-17  7:30 ` [PATCH 2/3] drm: replace the 'for' condition with outside defined variable John Hunter
2015-03-17  8:40   ` Daniel Vetter
2015-03-17  8:48     ` John Hunter
2015-03-17  9:24       ` Daniel Vetter
2015-03-17  9:29         ` John Hunter
2015-03-17  7:30 ` [PATCH 3/3] drm: change connector to tmp_connector John Hunter
2015-03-17  8:39   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.