All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] vlv sideband refactoring
@ 2013-05-15 18:40 Jani Nikula
  2013-05-15 18:40 ` [RFC PATCH 1/3] drm/i915: group vlv iosf sideband register accessors to a new file Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Jani Nikula @ 2013-05-15 18:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Okay, I'm stuck with this a bit, and whichever approach I choose I
expect there to be a bunch of bikeshedding. So I won't polish this
further before comments.

Both the intel_dpio_{read,write} and valleyview_{punit,nc}_{read,write}
use the IOSF sideband interface. They access the same registers and do
mostly the same stuff, but no shared code. There are even duplicate
register defines for the same registers. Both have locking, but the
former use dpio_lock and the latter rps.hw_lock. It's racy.

These patches refactor the sideband access to a single function that
expects dpio_lock to be held. The dpio_lock is only used for sideband
stuff, so it's a better match than rps.hw_lock for the purpose. The rps
stuff still needs rps.hw_lock, since it's used to protect more than just
the register access, so rps code will need to hold both locks.

The bikeshedding department:

1) Do we need to propagate sideband errors from the functions (like the
   valleyview_punit_* functions do), or, since the return values are
   never checked anywhere anyway, can we just convert to the
   intel_dpio_* style (functions return the register value only) for all
   of them?

2) There will be quite a few more ports. Add new wrappers for each of
   them, or create generic read/write functions that need a port
   argument?

3) Should the rps stuff take dpio_lock at a higher level than the
   wrappers? This is pretty much a requirement for the generic r/w
   function.

4) Does dpio really need a devfn different from the rest?

5) Your additional bikeshedding here. ;)


Cheers,
Jani.


Jani Nikula (3):
  drm/i915: group vlv iosf sideband register accessors to a new file
  drm/i915: refactor all vlv sideband accessors to use one helper
  drm/i915: drop redundant warnings on not holding dpio_lock

 drivers/gpu/drm/i915/Makefile         |    1 +
 drivers/gpu/drm/i915/i915_reg.h       |   93 ++++++++++++-------------
 drivers/gpu/drm/i915/intel_display.c  |   37 ----------
 drivers/gpu/drm/i915/intel_dp.c       |    6 --
 drivers/gpu/drm/i915/intel_hdmi.c     |    4 --
 drivers/gpu/drm/i915/intel_pm.c       |   60 ----------------
 drivers/gpu/drm/i915/intel_sideband.c |  121 +++++++++++++++++++++++++++++++++
 7 files changed, 165 insertions(+), 157 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sideband.c

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-05-21  8:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-15 18:40 [RFC PATCH 0/3] vlv sideband refactoring Jani Nikula
2013-05-15 18:40 ` [RFC PATCH 1/3] drm/i915: group vlv iosf sideband register accessors to a new file Jani Nikula
2013-05-21  8:04   ` Daniel Vetter
2013-05-15 18:40 ` [RFC PATCH 2/3] drm/i915: refactor all vlv sideband accessors to use one helper Jani Nikula
2013-05-15 18:40 ` [RFC PATCH 3/3] drm/i915: drop redundant warnings on not holding dpio_lock Jani Nikula
2013-05-21  8:33 ` [RFC PATCH 0/3] vlv sideband refactoring Daniel Vetter

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.