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@xxxxxxxx> 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@xxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
