On Mon, 8 Jan 2024 at 23:39, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 12/25/2023 5:08 AM, Dmitry Baryshkov wrote: > > dpu_encoder_phys_wb is the only user of encoder's atomic_check callback. > > Move corresponding checks to drm_writeback_connector's implementation > > and drop the dpu_encoder_phys_wb_atomic_check() function. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > --- > > .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 54 ------------------ > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++- > > drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 57 ++++++++++++++++++- > > drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h | 3 +- > > 4 files changed, 64 insertions(+), 59 deletions(-) > > > > I am fine with this change with respect to how the code is today. > > Hence, > > Reviewed-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> > > But if we start noticing a pattern like below in dpu_encoder.c's > atomic_check, > > if (INTF_WB) > ..... > else if (INTF_DP || INTF_DSI) > ..... > > then, it will demand bringing back a phys specific callback. The problem is that INTF_DP || INTF_DSI does not have the phys-specific implementation. So while I agree about the INTF_WB part, INTF_DP || INTF_DSI either should go as is, or it should be refactored into output-specific handlers. -- With best wishes Dmitry
