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