On Thu, Mar 28, 2019 at 11:18:56AM +0200, Jani Nikula wrote: > On Fri, 22 Mar 2019, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote: > > On Fri, Mar 22, 2019 at 09:28:01PM +0200, Jani Nikula wrote: > >> On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > >> > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: > >> >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: > >> >> > In that case there is no point in doing a rmw. > >> >> > >> >> But isnt it always a good idea to do rmw? I mean what if the master > >> >> select was set to something else earlier? > >> > > >> > RMW is the root of many evils. It should be avoided unless there is a > >> > really compelling reason to use it. > >> > >> Hear, hear! > >> > >> We have the software state that we want to write to the hardware. If we > >> use RMW to do this, it might all work by coincidence due to the old > >> values in the registers, or it might just as well break by coincidence > >> due to some garbage in the registers. > >> > >> In most cases, there should only be one place that writes a particular > >> display register during modeset. Sometimes this isn't possible, and RMW > >> is required. > >> > >> Some registers also have reserved bits potentially used by the hardware > >> that must not be changed, and RMW is required. These are documented in > >> bspec. > >> > >> BR, > >> Jani. > >> > > > > Thanks for the explanation. It does make sense now that we are doing a > > full modeset, we should just be then writing the value directly? The > > only concern I have is that say DSI code sets this somewhere els ein > > the modeset path, then we would need to modify this to do RMW or > > always make sure DSI also uses the same function for writing to this > > reg. What do you suggest doing now? > > I think all encoders in a tile group are always of the same type. Yes all the encoders in tile group are always same type. > > If the tile grouping in your patch is based purely on EDID, we may need > to enforce this. Surely genlock only works on encoders of the same type? > So all the slaves and their master will always be of same type and yes it is based on the EDID tile block parsing. But just to double sure I think when i assign the master slave pointers, I should check that the connector type is the same. > In any case DSI (at least currently) does not use tile groups, and will > never be mixed up in non-DSI tile groups. The DSI transcoders are > separate from other transcoders, so we're not writing the same registers > here. > > --- > > Looking at the code, I am wondering if this should be pushed to encoder > hooks instead of adding into crtc enable. As per the Bspec sequence, this needs to happen before enabling the TRANS_DDI_FUNC_CTL and after the link training, so I put in the crtc_enable hook, which encoder hooks are you suggesting adding this? Regards Manasi > > BR, > Jani. > > > > -- > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
