Re: [Intel-gfx] [PATCH 00/10] extract dp helper functions
On Thu, Oct 18, 2012 at 09:30:01AM -0400, Alex Deucher wrote: > On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter wrote: > > Hi all, > > > > I've frustrated myself the last few days yelling at our link training code. > > Comparing the i915 code to radeon and nouveau I've noticed the lack of a > > nice > > set of dp helper functions. So I've started to extract a few. > > > > There's lots more that we can do I think (link configuration selection, the > > i2c > > over aux retry stuff which diverges already between i915 and radeon, maybe > > more > > higher level parts of the training sequence). But there the drivers diverge > > quite a bit (e.g. the link configuration is driver by different things in > > each > > driver: coded link bw from the dp spec, link clock or required bw vs > > avialable > > bw), so that's more work and probably best done when reworking these > > functions > > for other reasons. > > > > I've also tried to put the new helpers a bit to use in nouveau, but due to > > lack > > of hw that part is untested. > > > > Comments and testing highly welcome. > > Looks good to me. Other than the minor change required in patch7, > this series is: > > Reviewed-by: Alex Deucher Slurped into drm-intel-next, with all the nouveau patches and hunks dropped (I'll pester Ben once this has landed in drm-next), as discussed on irc. Thanks for reviewing this pile. -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 00/10] extract dp helper functions
On Thu, Oct 18, 2012 at 3:48 PM, Alex Deucher wrote: > On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter wrote: >> Hi all, >> >> I've frustrated myself the last few days yelling at our link training code. >> Comparing the i915 code to radeon and nouveau I've noticed the lack of a nice >> set of dp helper functions. So I've started to extract a few. >> >> There's lots more that we can do I think (link configuration selection, the >> i2c >> over aux retry stuff which diverges already between i915 and radeon, maybe >> more >> higher level parts of the training sequence). But there the drivers diverge >> quite a bit (e.g. the link configuration is driver by different things in >> each >> driver: coded link bw from the dp spec, link clock or required bw vs >> avialable >> bw), so that's more work and probably best done when reworking these >> functions >> for other reasons. > > In theory we could provide a helper function to do the entire link > training in common code. We'd just need a a couple of function > callbacks: > > dp_aux_read() > dp_aux_write() > dp_link_train_init() > dp_set_src_training_pattern() > dp_link_train_fini() > > Obviously some drivers may want to do their own thing, so it would > just be a helper. Still for DP 1.2, it would be nice to have the > option of sharing more code. Yeah, that's one of the ideas. Although I think we should start with a few smaller things like e.g. the bandwidth stuff or the dp aux i2c logic. Those need a subset of the above interfaces only and are less intrusive. And since the link training in i915 seems to be still rather broken and it in decent flux due to hsw enabling in general, I want to wait and see a bit first until we have the corner cases nailed. -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 00/10] extract dp helper functions
On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter wrote: > Hi all, > > I've frustrated myself the last few days yelling at our link training code. > Comparing the i915 code to radeon and nouveau I've noticed the lack of a nice > set of dp helper functions. So I've started to extract a few. > > There's lots more that we can do I think (link configuration selection, the > i2c > over aux retry stuff which diverges already between i915 and radeon, maybe > more > higher level parts of the training sequence). But there the drivers diverge > quite a bit (e.g. the link configuration is driver by different things in each > driver: coded link bw from the dp spec, link clock or required bw vs avialable > bw), so that's more work and probably best done when reworking these functions > for other reasons. In theory we could provide a helper function to do the entire link training in common code. We'd just need a a couple of function callbacks: dp_aux_read() dp_aux_write() dp_link_train_init() dp_set_src_training_pattern() dp_link_train_fini() Obviously some drivers may want to do their own thing, so it would just be a helper. Still for DP 1.2, it would be nice to have the option of sharing more code. > > I've also tried to put the new helpers a bit to use in nouveau, but due to > lack > of hw that part is untested. > > Comments and testing highly welcome. > > Yours, Daniel > > Daniel Vetter (10): > drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c > drm: dp helper: extract drm_dp_channel_eq_ok > drm: dp helper: extract drm_dp_clock_recovery_ok > drm/nouveau: use the cr_ok/chanel_eq_ok helpers > drm: extract helpers to compute new training values from sink request > drm/nouveau: use dp link train request helper > drm: extract dp link train delay functions from radeon > drm/i915: use the new dp train delay helpers > drm: extract dp link bw helpers > drm: extract drm_dp_max_lane_count helper > > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_dp_helper.c | 328 > +++ > drivers/gpu/drm/drm_dp_i2c_helper.c | 208 -- > drivers/gpu/drm/i915/intel_dp.c | 98 ++- > drivers/gpu/drm/nouveau/nouveau_dp.c | 35 +--- > drivers/gpu/drm/radeon/atombios_dp.c | 147 ++-- > drivers/gpu/drm/radeon/radeon_mode.h | 2 +- > include/drm/drm_dp_helper.h | 31 > 8 files changed, 400 insertions(+), 451 deletions(-) > create mode 100644 drivers/gpu/drm/drm_dp_helper.c > delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c > > -- > 1.7.11.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/10] extract dp helper functions
On Thu, Oct 18, 2012 at 4:15 AM, Daniel Vetter wrote: > Hi all, > > I've frustrated myself the last few days yelling at our link training code. > Comparing the i915 code to radeon and nouveau I've noticed the lack of a nice > set of dp helper functions. So I've started to extract a few. > > There's lots more that we can do I think (link configuration selection, the > i2c > over aux retry stuff which diverges already between i915 and radeon, maybe > more > higher level parts of the training sequence). But there the drivers diverge > quite a bit (e.g. the link configuration is driver by different things in each > driver: coded link bw from the dp spec, link clock or required bw vs avialable > bw), so that's more work and probably best done when reworking these functions > for other reasons. > > I've also tried to put the new helpers a bit to use in nouveau, but due to > lack > of hw that part is untested. > > Comments and testing highly welcome. Looks good to me. Other than the minor change required in patch7, this series is: Reviewed-by: Alex Deucher > > Yours, Daniel > > Daniel Vetter (10): > drm: rename drm_dp_i2c_helper.c to drm_dp_helper.c > drm: dp helper: extract drm_dp_channel_eq_ok > drm: dp helper: extract drm_dp_clock_recovery_ok > drm/nouveau: use the cr_ok/chanel_eq_ok helpers > drm: extract helpers to compute new training values from sink request > drm/nouveau: use dp link train request helper > drm: extract dp link train delay functions from radeon > drm/i915: use the new dp train delay helpers > drm: extract dp link bw helpers > drm: extract drm_dp_max_lane_count helper > > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_dp_helper.c | 328 > +++ > drivers/gpu/drm/drm_dp_i2c_helper.c | 208 -- > drivers/gpu/drm/i915/intel_dp.c | 98 ++- > drivers/gpu/drm/nouveau/nouveau_dp.c | 35 +--- > drivers/gpu/drm/radeon/atombios_dp.c | 147 ++-- > drivers/gpu/drm/radeon/radeon_mode.h | 2 +- > include/drm/drm_dp_helper.h | 31 > 8 files changed, 400 insertions(+), 451 deletions(-) > create mode 100644 drivers/gpu/drm/drm_dp_helper.c > delete mode 100644 drivers/gpu/drm/drm_dp_i2c_helper.c > > -- > 1.7.11.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx