Re: [PATCH v1 1/3] drm/msm/dp: move struc of msm_display_info to msm_drv.h

[Kuogee Hsieh]

On 6/24/2022 2:40 PM, Doug Anderson wrote:
> Hi,
>
> On Fri, Jun 24, 2022 at 10:15 AM Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> wrote:
> > With current implementation, communication between interface driver and
> > upper mdss encoder layer are implemented through function calls. This
> > increase code complexity. Since struct msm_display_info contains msm
> > generic display information, it can be expended to contains more useful
> > information, such as widebus and dcs, in future to serve as communication
> > channel purpose between interface driver and upper mdss encoder layer so
> > that existing function calls can be eliminated.
> > This patch more struct msm_display_info to msm_drv.h to be visible by
> > whole msm scope.
> >
> > Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 20 --------------------
> >   drivers/gpu/drm/msm/msm_drv.h               | 19 +++++++++++++++++++
> >   2 files changed, 19 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > index 781d41c..6b604c5 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > @@ -19,26 +19,6 @@
> >   #define IDLE_TIMEOUT   (66 - 16/2)
> >
> >   /**
> > - * struct msm_display_info - defines display properties
> > - * @intf_type:          DRM_MODE_ENCODER_ type
> > - * @capabilities:       Bitmask of display flags
> > - * @num_of_h_tiles:     Number of horizontal tiles in case of split interface
> > - * @h_tile_instance:    Controller instance used per tile. Number of elements is
> > - *                      based on num_of_h_tiles
> > - * @is_te_using_watchdog_timer:  Boolean to indicate watchdog TE is
> > - *                              used instead of panel TE in cmd mode panels
> > - * @dsc:               DSC configuration data for DSC-enabled displays
> > - */
> > -struct msm_display_info {
> > -       int intf_type;
> > -       uint32_t capabilities;
> > -       uint32_t num_of_h_tiles;
> > -       uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
> > -       bool is_te_using_watchdog_timer;
> > -       struct msm_display_dsc_config *dsc;
> So in the structure you're "moving" there's this member called "dsc".
>
>
> > -};
> > -
> > -/**
> >    * dpu_encoder_assign_crtc - Link the encoder to the crtc it's assigned to
> >    * @encoder:   encoder pointer
> >    * @crtc:      crtc pointer
> > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> > index fdbaad5..f9c263b 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.h
> > +++ b/drivers/gpu/drm/msm/msm_drv.h
> > @@ -106,11 +106,30 @@ struct msm_drm_thread {
> >          struct kthread_worker *worker;
> >   };
> >
> > +<<<<<<< HEAD
> The above doesn't look like valid C to me.
>
>
> >   /* DSC config */
> >   struct msm_display_dsc_config {
> >          struct drm_dsc_config *drm;
> >   };
> >
> > +/**
> > + * struct msm_display_info - defines display properties
> > + * @intf_type:          DRM_MODE_ENCODER_ type
> > + * @capabilities:       Bitmask of display flags
> > + * @num_of_h_tiles:     Number of horizontal tiles in case of split interface
> > + * @h_tile_instance:    Controller instance used per tile. Number of elements is
> > + *                      based on num_of_h_tiles
> > + * @is_te_using_watchdog_timer:  Boolean to indicate watchdog TE is
> > + *                              used instead of panel TE in cmd mode panels
> > + */
> > +struct msm_display_info {
> > +       int intf_type;
> > +       uint32_t capabilities;
> > +       uint32_t num_of_h_tiles;
> > +       uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
> > +       bool is_te_using_watchdog_timer;
> ...but then when you "move" the structure to its new location, which
> should be a noop, then <poof> the "dsc" variable vanishes (along with
> the kernel doc description of it before the structure).

Sorry, i did not resolve the conflicts correctly  when i cherry-pick 
them to msm-next tree.

Will fix them.

> -Doug