On Fri, Feb 28, 2014 at 07:09:51PM -0300, Paulo Zanoni wrote: > 2014-02-27 9:23 GMT-03:00 <ville.syrjala@xxxxxxxxxxxxxxx>: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > On DDI there's no PLL as such to generate the pixel clock for VGA. > > Instead we derive the pixel clock from the FDI link frequency. So > > to make .compute_config match what .get_config does, we need to > > set the port_clock based on the FDI link frequency. > > > > Note that we don't even check the port_clock when selecting the > > PLL for VGA output. We just assume SPLL at 1.35GHz is what we want, > > and that does match with the asumption of FDI frequency of 2.7Ghz > > we have in intel_fdi_link_freq(). > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74955 > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_crt.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > > index 9864aa1..02e2b02 100644 > > --- a/drivers/gpu/drm/i915/intel_crt.c > > +++ b/drivers/gpu/drm/i915/intel_crt.c > > @@ -262,6 +262,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder, > > if (HAS_PCH_LPT(dev)) > > pipe_config->pipe_bpp = 24; > > > > + /* FDI must always be 2.7 GHz */ > > + if (HAS_DDI(dev)) > > + pipe_config->port_clock = 135000 * 2; > > + > > <rant> > Whenever I have to review patches touching the HW state > readout/compute I get completely confused. We have a bunch of > compute_config/get_config/get_hw_state functions, with no real > documentation of which fields exactly each function should get. I have no objections to improving the docs. > Our > hardware also does a bunch of *2 multiplications on PLLs and clocks, > and this gets things even more confusing. The 2x is for PLL output->bitclock, and the /5 would be for PLL output->symbol clock. So maybe it would be nicer to use <real PLL frequency>/5 in the DDI get_config. It would match how we compute things on VLV at least. > Also, we have similar > information (like all these "clocks", from mode, adjusted_mode, port, > pipe, hw, etc) with slightly different meanings. I really wish we had > Kernel documentation describing *exactly* what is expected to be > computed from each function, and what are the differences between all > those similar things with slightly different meanings. Maybe we could > also organize struct intel_crtc_config, adding small sub-structs that > better define the domain of each field? > </rant> > > Still, I have reviewed your patch, and despite all my own confusions, > it looks correct, so: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > > return true; > > } > > > > -- > > 1.8.3.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
