All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 1/3] drm/i915: add PCH_NONE to enum intel_pch
Date: Tue,  3 Jul 2012 18:48:16 -0300	[thread overview]
Message-ID: <1341352096-19712-1-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1341341853-4092-1-git-send-email-przanoni@gmail.com>

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

And rely on the fact that it's 0 to assume that machines without a PCH
will have PCH_NONE as dev_priv->pch_type.

Just today I finally realized that HAS_PCH_IBX is true for machines
without a PCH. IMHO this is totally counter-intuitive and I don't
think it's a good idea to assume that we're going to check for
HAS_PCH_IBX only after we check for HAS_PCH_SPLIT.

I believe that in the future we'll have more PCH types and checks
like:

    if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))

will become more and more common. There's a good chance that we may
break non-PCH machines by adding these checks in code that runs on all
machines. I also believe that the HAS_PCH_SPLIT check will become less
common as we add more and more different PCH types. We'll probably
start replacing checks like:

    if (HAS_PCH_SPLIT(dev))
        foo();
    else
        bar();

with:

    if (HAS_PCH_NEW(dev))
        baz();
    else if (HAS_PCH_OLD(dev) || HAS_PCH_IBX(dev))
        foo();
    else
        bar();

and this may break gen 2/3/4.

As far as we have investigated, this patch will affect the behavior of
intel_hdmi_dpms and intel_dp_link_down on gen 4. In both functions the
code inside the HAS_PCH_IBX check is for IBX-specific workarounds, so
we should be safe. If we start bisecting gen 2/3/4 bugs to this commit
we should consider replacing the HAS_PCH_IBX checks with something
else.

V2: Improve commit message, list possible side effects and solution.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 1 file changed, 1 insertion(+)

Another alternative would have been to change HAS_PCH_IBX to also
check for HAS_PCH_SPLIT, but I'm not exactly in favor of adding more
conditionals...

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7a1eaa..b12e79a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -333,6 +333,7 @@ enum no_fbc_reason {
 };
 
 enum intel_pch {
+	PCH_NONE = 0,	/* No PCH present */
 	PCH_IBX,	/* Ibexpeak PCH */
 	PCH_CPT,	/* Cougarpoint PCH */
 	PCH_LPT,	/* Lynxpoint PCH */
-- 
1.7.10.2

  parent reply	other threads:[~2012-07-03 21:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-03 18:57 [PATCH 1/3] drm/i915: add PCH_NONE to enum intel_pch Paulo Zanoni
2012-07-03 18:57 ` [PATCH 2/3] drm/i915: get rid of dev_priv->info->has_pch_split Paulo Zanoni
2012-07-03 19:15   ` Daniel Vetter
2012-07-03 20:46     ` Paulo Zanoni
2012-07-03 18:57 ` [PATCH 3/3] drm/i915: don't ironlake_init_pch_refclk() on LPT Paulo Zanoni
2012-07-03 19:10 ` [PATCH 1/3] drm/i915: add PCH_NONE to enum intel_pch Daniel Vetter
2012-07-03 20:29   ` Paulo Zanoni
2012-07-03 20:39     ` Daniel Vetter
2012-07-03 21:48 ` Paulo Zanoni [this message]
2012-07-04  7:49   ` Daniel Vetter

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=1341352096-19712-1-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.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.