Re: [Intel-gfx] [PATCH 0/5] drm/i915 Avoid long delays when reading EDID on eDP
On Tue, Nov 25, 2014 at 06:20:17PM +0100, Egbert Eich wrote:
Daniel Vetter writes:
Imo this approach with overwrite all the entry points won't scale since
besides i2c and dpcd there will be more sooner or later (oui, dp mst, some
debugfs userspace dp aux tools, ...).
I think what we need is the same as in the i2c layer has with the
xfer_pre/post functions. To make this as painless as possible we should
probably refcount that in the dp helper, protected by aux-hw_mutex. That
way normal dp reads could just do the xfer_pre/post around the call to
aux-transfer while i2c could do it around the entire i2c transaction.
I agree with your idea of having xfer_pre/post functions.
I'm not sure though if I understand your idea about reference counters:
are you trying to protect from xfer_pre/post being called at different
levels here?
With my proposal entire transactions (I2C) are serialized thru the pps_mutex
lock. This is a side effect that's probably unwanted. Is this what you are
trying to avoid by using ref counters?
Yeah I want to protect entire transactions at the highest levels with
xfer_pre/post. The problem is that things nest and we can't grab
sufficient locking to prevent that nesting. E.g. for i2c we should do the
dp_aux xfer_pre/post calls from i2c xfer_pre/post (so that the entire edid
read only has one pre/post, assuming no retries). But the i2c lock which
protects that doesn't protect against concurrent dp aux transactions from
someone else (e.g. dp mst is done without modeset locks, or if we ever get
a userspace interface for debuggin db issues). So direct dp aux again
needs to do xfer_pre/post. Same for dp mst, we might want to wrap an
entire dp mst transaction with this, but the low-level code might not now.
Hence why I think we shold have a refcount for pre/post plus small helpers
(atm just static, we can export once we need them) like this
drm_dp_aux_xfer_pre(dp_aux)
{
mutex_lock(dp_aux-hw_lock)
if (dp_aux-xfer_count++ == 0)
dp_aux-xfer_pre();
mutex_unlock(dp_aux-hw_lock)
}
Same for xfer_post. Then we could wire that up for i2c-over-dp and for the
low-level dp xfer functions and it should all Just Work. And if there's a
new perf critical case where we want to do this over an entire series of
transactinos we can just call the two xfer_pre/post wrappers and done.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/5] drm/i915 Avoid long delays when reading EDID on eDP
On Mon, 24 Nov 2014, Egbert Eich e...@suse.de wrote:
For eDP in the Intel driver pps_lock()/unlock() need to be called before
initiating an I2C/AUX channel transfer. These operations can be quite
expensive - especially on values for HZ lower than 1000.
It is therefore better to perfrom this locking/unlocking only once,
ie at the beginning and at the end of the entire I2C transfer.
The current design of drm_dp_helper.c doesn't allow this.
This patchset modifies drm_dp_helper.c and moves the locking/unlocking
operation to the top.
This fixes the long delay observed in
https://bugs.freedesktop.org/show_bug.cgi?id=86201
Egbert Eich (4):
drm/DP: Create pointer to generic DPCD access function
drm/DP: Export drm_dp_i2c_xfer() DP helper function
drm/DP: Export drm_dp_dpcd_access() DP helper function
These three need to be sent to dri-devel, with cc: Thierry Reding
thierry.red...@gmail.com.
BR,
Jani.
drm/i915/eDP: Move pps_lock() and edp_panel_vdd_on() to top
Ville Syrjälä (1):
drm/i915: Try to avoid pps_{lock,unlock}() on DP ports
drivers/gpu/drm/drm_dp_helper.c | 11 ++--
drivers/gpu/drm/i915/intel_dp.c | 132
+++
drivers/gpu/drm/i915/intel_drv.h | 5 ++
include/drm/drm_dp_helper.h | 14 +
4 files changed, 133 insertions(+), 29 deletions(-)
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/5] drm/i915 Avoid long delays when reading EDID on eDP
On Mon, Nov 24, 2014 at 06:16:22PM +0100, Egbert Eich wrote:
For eDP in the Intel driver pps_lock()/unlock() need to be called before
initiating an I2C/AUX channel transfer. These operations can be quite
expensive - especially on values for HZ lower than 1000.
It is therefore better to perfrom this locking/unlocking only once,
ie at the beginning and at the end of the entire I2C transfer.
The current design of drm_dp_helper.c doesn't allow this.
This patchset modifies drm_dp_helper.c and moves the locking/unlocking
operation to the top.
This fixes the long delay observed in
https://bugs.freedesktop.org/show_bug.cgi?id=86201
Egbert Eich (4):
drm/DP: Create pointer to generic DPCD access function
drm/DP: Export drm_dp_i2c_xfer() DP helper function
drm/DP: Export drm_dp_dpcd_access() DP helper function
drm/i915/eDP: Move pps_lock() and edp_panel_vdd_on() to top
Ville Syrjälä (1):
drm/i915: Try to avoid pps_{lock,unlock}() on DP ports
Imo this approach with overwrite all the entry points won't scale since
besides i2c and dpcd there will be more sooner or later (oui, dp mst, some
debugfs userspace dp aux tools, ...).
I think what we need is the same as in the i2c layer has with the
xfer_pre/post functions. To make this as painless as possible we should
probably refcount that in the dp helper, protected by aux-hw_mutex. That
way normal dp reads could just do the xfer_pre/post around the call to
aux-transfer while i2c could do it around the entire i2c transaction.
-Daniel
drivers/gpu/drm/drm_dp_helper.c | 11 ++--
drivers/gpu/drm/i915/intel_dp.c | 132
+++
drivers/gpu/drm/i915/intel_drv.h | 5 ++
include/drm/drm_dp_helper.h | 14 +
4 files changed, 133 insertions(+), 29 deletions(-)
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/5] drm/i915 Avoid long delays when reading EDID on eDP
Daniel Vetter writes: Imo this approach with overwrite all the entry points won't scale since besides i2c and dpcd there will be more sooner or later (oui, dp mst, some debugfs userspace dp aux tools, ...). I think what we need is the same as in the i2c layer has with the xfer_pre/post functions. To make this as painless as possible we should probably refcount that in the dp helper, protected by aux-hw_mutex. That way normal dp reads could just do the xfer_pre/post around the call to aux-transfer while i2c could do it around the entire i2c transaction. I agree with your idea of having xfer_pre/post functions. I'm not sure though if I understand your idea about reference counters: are you trying to protect from xfer_pre/post being called at different levels here? With my proposal entire transactions (I2C) are serialized thru the pps_mutex lock. This is a side effect that's probably unwanted. Is this what you are trying to avoid by using ref counters? Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] drm/i915 Avoid long delays when reading EDID on eDP
For eDP in the Intel driver pps_lock()/unlock() need to be called before
initiating an I2C/AUX channel transfer. These operations can be quite
expensive - especially on values for HZ lower than 1000.
It is therefore better to perfrom this locking/unlocking only once,
ie at the beginning and at the end of the entire I2C transfer.
The current design of drm_dp_helper.c doesn't allow this.
This patchset modifies drm_dp_helper.c and moves the locking/unlocking
operation to the top.
This fixes the long delay observed in
https://bugs.freedesktop.org/show_bug.cgi?id=86201
Egbert Eich (4):
drm/DP: Create pointer to generic DPCD access function
drm/DP: Export drm_dp_i2c_xfer() DP helper function
drm/DP: Export drm_dp_dpcd_access() DP helper function
drm/i915/eDP: Move pps_lock() and edp_panel_vdd_on() to top
Ville Syrjälä (1):
drm/i915: Try to avoid pps_{lock,unlock}() on DP ports
drivers/gpu/drm/drm_dp_helper.c | 11 ++--
drivers/gpu/drm/i915/intel_dp.c | 132 +++
drivers/gpu/drm/i915/intel_drv.h | 5 ++
include/drm/drm_dp_helper.h | 14 +
4 files changed, 133 insertions(+), 29 deletions(-)
--
1.8.4.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
