On 01/27/14 15:31, Ville Syrjälä wrote: > On Mon, Jan 27, 2014 at 03:09:34PM +0200, Antti Koskipaa wrote: >> + .dpll_offsets = { DPLL_A_OFFSET, DPLL_B_OFFSET }, \ >> + .dpll_md_offsets = { DPLL_A_MD_OFFSET, DPLL_B_MD_OFFSET }, \ >> + .palette_offsets = { PALETTE_A_OFFSET, PALETTE_B_OFFSET }, \ > > Please drop the last comma here ... > >> + >> + >> static const struct intel_device_info intel_i830_info = { >> .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, >> .has_overlay = 1, .overlay_needs_physical = 1, >> .ring_mask = RENDER_RING, >> + GEN_DEFAULT_PIPEOFFSETS > > ... and place it here > Done. >> +/* >> + * There's actually no pipe EDP. Some pipe registers have >> + * simply shifted from the pipe to the transcoder, while >> + * keeping their original offset. Thus we need PIPE_EDP_OFFSET >> + * to access such registers in transcoder EDP. >> + */ >> +#define PIPE_EDP_OFFSET 0x7f000 > > I just realized that you're not using this actually. But I think we need > to use it to make *future* stuff work. > > For the same reason PIPECONF() needs to use _PIPE2() macro, BCLRPAT needs > to use _TRANSCODER2() macro. Ie. the choice of which we use needs to be > based strictly on the offset range where the register lives. > 0x6xxxx -> _TRANSCODER2() > 0x7xxxx -> _PIPE2() > > PIPESRC() seems to be where it needs to be, ie. using _TRANCODER2(). Read it again. The old code has PIPECONF and BCLRPAT using the wrong macro. This patch fixes it. > So this also means we need the pipe_offsets[] array to have space for > PIPE_EDP_OFFSET. Done. -- - Antti _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx