| Message ID | 1368704437-17034-3-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: > On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we > can calculate for both the clock divider for the 2MHz target rate at the > same place. Afterwards we can also replace the is_cpu_edp() check with a > check for port A. > > Signed-off-by: Imre Deak <imre.deak@intel.com> There's a now-dead IS_VLV case in intel_hrawclk which should be killed with this patch, too. -Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 90ae58a..b99da13 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -317,11 +317,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > * Note that PCH attached eDP panels should use a 125MHz input > * clock divider. > */ > - if (is_cpu_edp(intel_dp)) { > + if (IS_VALLEYVIEW(dev)) { > + aux_clock_divider = 100; > + } else if (intel_dig_port->port == PORT_A) { > if (HAS_DDI(dev)) > aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1; > - else if (IS_VALLEYVIEW(dev)) > - aux_clock_divider = 100; > else if (IS_GEN6(dev) || IS_GEN7(dev)) > aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */ > else > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: > On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: > > On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we > > can calculate for both the clock divider for the 2MHz target rate at the > > same place. Afterwards we can also replace the is_cpu_edp() check with a > > check for port A. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > There's a now-dead IS_VLV case in intel_hrawclk which should be killed > with this patch, too. Shouldn't it still return the correct value if someone calls it in the future? Actually it's also used in intel_dp_init_panel_power_sequencer_registers(). --Imre > -Daniel > > > --- > > drivers/gpu/drm/i915/intel_dp.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 90ae58a..b99da13 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -317,11 +317,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > > * Note that PCH attached eDP panels should use a 125MHz input > > * clock divider. > > */ > > - if (is_cpu_edp(intel_dp)) { > > + if (IS_VALLEYVIEW(dev)) { > > + aux_clock_divider = 100; > > + } else if (intel_dig_port->port == PORT_A) { > > if (HAS_DDI(dev)) > > aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1; > > - else if (IS_VALLEYVIEW(dev)) > > - aux_clock_divider = 100; > > else if (IS_GEN6(dev) || IS_GEN7(dev)) > > aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */ > > else > > -- > > 1.7.10.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Tue, May 21, 2013 at 12:36 PM, Imre Deak <imre.deak@intel.com> wrote: > On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: >> On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: >> > On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we >> > can calculate for both the clock divider for the 2MHz target rate at the >> > same place. Afterwards we can also replace the is_cpu_edp() check with a >> > check for port A. >> > >> > Signed-off-by: Imre Deak <imre.deak@intel.com> >> >> There's a now-dead IS_VLV case in intel_hrawclk which should be killed >> with this patch, too. > > Shouldn't it still return the correct value if someone calls it in the > future? Actually it's also used in > intel_dp_init_panel_power_sequencer_registers(). Indeed. I think though it's better to make this an explicit vlv case since hrawclk talks about the FSB. And that thing pretty surely doesn't exist on vlv any more ;-) So maybe a follow-up patch? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, 2013-05-21 at 12:42 +0200, Daniel Vetter wrote: > On Tue, May 21, 2013 at 12:36 PM, Imre Deak <imre.deak@intel.com> wrote: > > On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: > >> On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: > >> > On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we > >> > can calculate for both the clock divider for the 2MHz target rate at the > >> > same place. Afterwards we can also replace the is_cpu_edp() check with a > >> > check for port A. > >> > > >> > Signed-off-by: Imre Deak <imre.deak@intel.com> > >> > >> There's a now-dead IS_VLV case in intel_hrawclk which should be killed > >> with this patch, too. > > > > Shouldn't it still return the correct value if someone calls it in the > > future? Actually it's also used in > > intel_dp_init_panel_power_sequencer_registers(). > > Indeed. I think though it's better to make this an explicit vlv case > since hrawclk talks about the FSB. And that thing pretty surely > doesn't exist on vlv any more ;-) Ok. Tbh, I haven't thought about the differences in clock topology across the platforms, but would be nice to understand it better. > So maybe a follow-up patch? Yep, I think it could stand on its own. --Imre
On Tue, 2013-05-21 at 13:59 +0300, Imre Deak wrote: > On Tue, 2013-05-21 at 12:42 +0200, Daniel Vetter wrote: > > On Tue, May 21, 2013 at 12:36 PM, Imre Deak <imre.deak@intel.com> wrote: > > > On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: > > >> On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: > > >> > On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we > > >> > can calculate for both the clock divider for the 2MHz target rate at the > > >> > same place. Afterwards we can also replace the is_cpu_edp() check with a > > >> > check for port A. > > >> > > > >> > Signed-off-by: Imre Deak <imre.deak@intel.com> > > >> > > >> There's a now-dead IS_VLV case in intel_hrawclk which should be killed > > >> with this patch, too. > > > > > > Shouldn't it still return the correct value if someone calls it in the > > > future? Actually it's also used in > > > intel_dp_init_panel_power_sequencer_registers(). > > > > Indeed. I think though it's better to make this an explicit vlv case > > since hrawclk talks about the FSB. And that thing pretty surely > > doesn't exist on vlv any more ;-) > > Ok. Tbh, I haven't thought about the differences in clock topology > across the platforms, but would be nice to understand it better. I checked now and the VLV spec does define hrawclk, so that would justify keeping it in intel_hrawclk. --Imre
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 90ae58a..b99da13 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -317,11 +317,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, * Note that PCH attached eDP panels should use a 125MHz input * clock divider. */ - if (is_cpu_edp(intel_dp)) { + if (IS_VALLEYVIEW(dev)) { + aux_clock_divider = 100; + } else if (intel_dig_port->port == PORT_A) { if (HAS_DDI(dev)) aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1; - else if (IS_VALLEYVIEW(dev)) - aux_clock_divider = 100; else if (IS_GEN6(dev) || IS_GEN7(dev)) aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */ else
On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we can calculate for both the clock divider for the 2MHz target rate at the same place. Afterwards we can also replace the is_cpu_edp() check with a check for port A. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)