All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: Supporting fused display configurations v4
Date: Tue, 7 Jan 2014 09:07:10 +0100	[thread overview]
Message-ID: <20140107080710.GF4770@phenom.ffwll.local> (raw)
In-Reply-To: <CA+gsUGSQf8YPegJxyyiRU+0KkYv7aRRFeYgrtNefKEzNvKNhSA@mail.gmail.com>

On Mon, Jan 06, 2014 at 08:05:56PM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2014/1/6 Damien Lespiau <damien.lespiau@intel.com>:
> > Follow up of the v3:
> >   http://lists.freedesktop.org/archives/intel-gfx/2013-December/037333.html
> >
> > The main change is the removal of INTEL_INFO() from the driver (patches 1 to
> > 5), a clean-up suggested by Chris: the usage of that macro hides that we go
> > from dev to dev_priv while we could use the dev_priv pointer that is usually
> > available.
> >
> > Patch 6 is also new, a minor clean-up while at it.
> 
> For patches 1 to 6:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 
> Some comments/bikesheds (I'm not requiring these changes, just
> throwing some ideas):
> 
> - You removed INTEL_INFO, but all those IS_SOMETHING and HAS_SOMETHING
> macros still accept dev as argument instead of dev_priv. Do we have
> plans to change this too? I can remember many places where I had to
> add a "dev" variable just because of these macros. Perhaps maybe the
> new goal is a series removing the to_i915 macro? I could see a lot of
> code getting almost entirely rid of "dev" with these changes.

Once we have proper subclassing casting between drm_device and i915_device
is very cheap, so I'm ok with sprinkling to_i915 all over those macros. If
we end up with tons of IS_SOMETHING(&dev_priv->base) kind of code in a
glorious future we can then switch things around.

> - It would be nice if we could come up with a clever strategy to find
> out if a still-not-initialized field of intel_device_info is being
> used. No clever ideas here, besides the obvious implementations that
> don't look good.

Same here - would be neat but no idea.

> - Quick! Merge patch 3 before it conflicts and someone needs to review
> that giant thing again!

As long as topic/ppgtt is still hanging out there and not yet merged into
dinq that can't happen due to conflict galore. And since there are still
some regressions pending that'll take a while.

Please don't blame me for this mess, I never really liked this approach of
merging ppgtt ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2014-01-07  8:05 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-06 19:17 Supporting fused display configurations v4 Damien Lespiau
2014-01-06 19:17 ` [PATCH 01/12] drm/i915: Get rid of the INTEL_INFO() usage in i915_drv.h Damien Lespiau
2014-01-06 19:17 ` [PATCH 02/12] drm/i915: Convert a few WM functions to use struct drm_i915_private directly Damien Lespiau
2014-01-06 19:17 ` [PATCH 03/12] drm/i915: Mass replace INTEL_INFO() by dev_priv->info Damien Lespiau
2014-01-06 19:17 ` [PATCH 04/12] drm/i915: Finish replacing INTEL_INFO() by direct dev_priv usage Damien Lespiau
2014-01-06 19:17 ` [PATCH 05/12] drm/i915: Remove INTEL_INFO() Damien Lespiau
2014-01-26 15:09   ` Daniel Vetter
2014-01-06 19:17 ` [PATCH 06/12] drm/i915: Constify the drm_i915_private pointer a bit more Damien Lespiau
2014-01-06 19:17 ` [PATCH 07/12] drm/i915: Make the intel_device_info structure kept in dev_priv writable Damien Lespiau
2014-01-06 19:17 ` [PATCH 08/12] drm/i915: Move num_plane to the intel_device_info structure Damien Lespiau
2014-01-06 19:17 ` [PATCH 09/12] drm/i915: Consolidate FUSE_STRAP in one set of defines Damien Lespiau
2014-01-06 19:17 ` [PATCH 10/12] drm/i915: Disable display when fused off Damien Lespiau
2014-01-06 19:17 ` [PATCH 11/12] drm/i915: Remove the Quanta special case Damien Lespiau
2014-01-06 19:17 ` [PATCH 12/12] drm/i915: Use I915_MAX_PIPES in the pipe/plane_to_crtc_mapping definitions Damien Lespiau
2014-01-06 22:05 ` Supporting fused display configurations v4 Paulo Zanoni
2014-01-07  8:07   ` Daniel Vetter [this message]
2014-01-07 16:55     ` Damien Lespiau
2014-01-07  8:23   ` Jani Nikula
2014-01-07 10:00     ` Chris Wilson

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=20140107080710.GF4770@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=przanoni@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.