Intel-GFX Archive on 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 15:57:31 -0300	[thread overview]
Message-ID: <1341341853-4092-1-git-send-email-przanoni@gmail.com> (raw)

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.

Also: are we sure we don't already have any bugs triggered by checking
for HAS_PCH_IBX on non-PCH machines?

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

             reply	other threads:[~2012-07-03 18:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-03 18:57 Paulo Zanoni [this message]
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
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=1341341853-4092-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox