On 30/01/2023 23:51, Abhinav Kumar wrote:
On 12/29/2022 11:18 AM, Dmitry Baryshkov wrote:
In preparation to adding fully virtualized planes, move struct
dpu_hw_sspp instance from struct dpu_plane to struct dpu_plane_state, as
it will become a part of state (allocated during atomic check) rather
than part of a plane (allocated during boot).
I was thinking about a couple of things about this patch:
1) Since we are moving away from using "pipe" and using "sspp", perhaps
we can rename pipe_hw to hw_sspp in the below struct
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -35,6 +35,8 @@ struct dpu_plane_state {
uint32_t multirect_mode;
bool pending;
+ struct dpu_hw_pipe *hw_sspp;
+
Ack
u64 plane_fetch_bw;
u64 plane_clk;
};
2) I still dont see any comment as promised in v1 about why we are doing
this in dpu_plane_reset().
https://patchwork.freedesktop.org/patch/473155/?series=99909&rev=1#comment_875365
I think what we need to mention is that the dpu_plane_reset() is the one
which allocates the plane state today and hence pipe_hw can only be
assigned there.
Ack
Rest LGTM.
--
With best wishes
Dmitry
