All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Ilija Hadzic <ilijahadzic@gmail.com>
Cc: Ilija Hadzic <ihadzic@research.bell-labs.com>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: allocate crtc mutex separately from crtc
Date: Sat, 19 Oct 2013 12:41:08 +0200	[thread overview]
Message-ID: <20131019104108.GA4830@phenom.ffwll.local> (raw)
In-Reply-To: <CA+4h6HnA-w-RV0rSOB6UsoJAhGihKLK1Ja-qbON4DGDuwQBnXQ@mail.gmail.com>

On Thu, Oct 17, 2013 at 09:27:41PM -0400, Ilija Hadzic wrote:
> (dropping stable@... until we get the fix we can agree on)
> 
> While looking through that function (drm_crtc_helper_set_config) to
> figure out what we really need to save and restore, I came across a
> piece of code that you added in 25f397a4. The 'if (connector->dpms !=
> DRM_MODE_DPMS_ON)'  (line 719 in the version that is on the top of
> Dave's drm-next branch), comes right after the unconditional break, is
> unreachable code (and removing it produces the same object code). Can
> you explain what your intent was here? Also, the comment above the
> break that reads "don't break so fail path works correct[sic]" doesn't
> make much sense to me either.

The idea was to also remove the break there. I just haven't had the time
yet to fix this fumble and test it a bit.

For the funny commment I think this is just to avoid the code in the outer
for loop wrestling with the connector->encoder pointer. Removing the
comment and inline the ret = -EINVAL; goto fail; would be clearer code
imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

      reply	other threads:[~2013-10-19 10:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-16 23:35 [PATCH] drm: allocate crtc mutex separately from crtc Ilija Hadzic
2013-10-17  7:16 ` Daniel Vetter
2013-10-17 12:58   ` Ilija Hadzic
2013-10-18  1:27   ` Ilija Hadzic
2013-10-19 10:41     ` Daniel Vetter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131019104108.GA4830@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ihadzic@research.bell-labs.com \
    --cc=ilijahadzic@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.