On 2022-12-16 14:20:52, Abhinav Kumar wrote: > > > On 12/14/2022 5:08 PM, Dmitry Baryshkov wrote: > > On 14/12/2022 21:30, Marijn Suijten wrote: > >> On 2022-12-14 20:43:29, Dmitry Baryshkov wrote: > >>> On 14/12/2022 01:22, Marijn Suijten wrote: > >>>> [..] > >>> We usually don't have DSC with the writeback, don't we? > >> > >> I am unsure so ended up adding them in writeback regardless. Downstream > >> uses a separate callback to process intf_cfg.dsc instead of going > >> through setup_intf_cfg(). > >> > >> To prevent these from being missed again (in the case of copy&paste), > >> how about instead having some function that sets up intf_cfg with these > >> default values from a phys_enc? That way most of this remains oblivious > >> to the caller. > > > > I'm not sure this is possible. E.g. intf_cfg.dsc should not be set for > > the WB. > > > > Although this change is harmless because > dpu_encoder_helper_get_dsc(phys_enc) will not return a valid DSC mask > for the WB encoder, hence the setup_intf_cfg will just skip the DSC > programming, I also agree that we can skip setting the intf_cfg.dsc for > the writeback encoder in this patch. Since both of you agree that it is useless I'll drop this in V2. Have to confess that I know nothing about the writeback interface and haven't even read the code; does it run in parallel to a "physical" (e.g. DP/DSI) interface to capture screenshots (or even video) of what is currently being shown on the screen? By that logic the WB may have needed to know what is going on in the HW, but it wouldn't have made any sense regardless if the presented planes first pass through DSC before being captured. Something for me to read up on :) - Marijn
