From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Kuogee Hsieh <quic_khsieh@quicinc.com> Cc: dri-devel@lists.freedesktop.org, robdclark@gmail.com, sean@poorly.run, swboyd@chromium.org, dianders@chromium.org, vkoul@kernel.org, daniel@ffwll.ch, airlied@linux.ie, agross@kernel.org, bjorn.andersson@linaro.org, quic_abhinavk@quicinc.com, quic_aravindh@quicinc.com, quic_sbillaka@quicinc.com, freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 1/3] drm/msm/dp: move struc of msm_display_info to msm_drv.h Date: Sat, 25 Jun 2022 02:41:35 +0300 [thread overview] Message-ID: <CAA8EJpozw5r657SaohnFzNzg59sO41+Q2qJkcjxDmCNCPnVe0w@mail.gmail.com> (raw) In-Reply-To: <1656090912-18074-2-git-send-email-quic_khsieh@quicinc.com> On Fri, 24 Jun 2022 at 20:15, Kuogee Hsieh <quic_khsieh@quicinc.com> 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. NAK. The msm_display_info contains information used by (and useful to) DPU only, it is not 'msm generic' info. For this reason it has been moved from msm_drv.h to dpu_encoder.h inIn commit b7420739f112 ("drm/msm: move struct msm_display_info to dpu driver") . Neither mdp5 nor mdp4 are going to use this structure. At some point I thought too that we might be able to create a set of data and functions to describe encoder backends (dsi, hdmi, dp). This has failed for me. After musing over the msm_drv.h part containing functions published by the backends, I could not end up with a set of them being good enough. The only common part seems to be the modeset_init, snapshot and (once DP gets the DSC interface) get_dsc_config. The rest is backend-specific. I would suggest returning to this topic if/once DSI gets wide bus support or DP starts using bonded interfaces. Before that I don't foresee that common data structure would simplify things rather than complicating them. > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > 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; > -}; > - > -/** > * 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 Not to mention that this patch is broken per se. > /* 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; > +}; > + > struct msm_drm_private { > > struct drm_device *dev; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- With best wishes Dmitry
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Kuogee Hsieh <quic_khsieh@quicinc.com> Cc: quic_sbillaka@quicinc.com, quic_abhinavk@quicinc.com, airlied@linux.ie, freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, vkoul@kernel.org, agross@kernel.org, bjorn.andersson@linaro.org, linux-arm-msm@vger.kernel.org, quic_aravindh@quicinc.com, swboyd@chromium.org, sean@poorly.run, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 1/3] drm/msm/dp: move struc of msm_display_info to msm_drv.h Date: Sat, 25 Jun 2022 02:41:35 +0300 [thread overview] Message-ID: <CAA8EJpozw5r657SaohnFzNzg59sO41+Q2qJkcjxDmCNCPnVe0w@mail.gmail.com> (raw) In-Reply-To: <1656090912-18074-2-git-send-email-quic_khsieh@quicinc.com> On Fri, 24 Jun 2022 at 20:15, Kuogee Hsieh <quic_khsieh@quicinc.com> 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. NAK. The msm_display_info contains information used by (and useful to) DPU only, it is not 'msm generic' info. For this reason it has been moved from msm_drv.h to dpu_encoder.h inIn commit b7420739f112 ("drm/msm: move struct msm_display_info to dpu driver") . Neither mdp5 nor mdp4 are going to use this structure. At some point I thought too that we might be able to create a set of data and functions to describe encoder backends (dsi, hdmi, dp). This has failed for me. After musing over the msm_drv.h part containing functions published by the backends, I could not end up with a set of them being good enough. The only common part seems to be the modeset_init, snapshot and (once DP gets the DSC interface) get_dsc_config. The rest is backend-specific. I would suggest returning to this topic if/once DSI gets wide bus support or DP starts using bonded interfaces. Before that I don't foresee that common data structure would simplify things rather than complicating them. > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > 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; > -}; > - > -/** > * 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 Not to mention that this patch is broken per se. > /* 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; > +}; > + > struct msm_drm_private { > > struct drm_device *dev; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- With best wishes Dmitry
next prev parent reply other threads:[~2022-06-24 23:41 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-06-24 17:15 [PATCH v1 0/3] fix primary corruption issue Kuogee Hsieh 2022-06-24 17:15 ` Kuogee Hsieh 2022-06-24 17:15 ` [PATCH v1 1/3] drm/msm/dp: move struc of msm_display_info to msm_drv.h Kuogee Hsieh 2022-06-24 17:15 ` Kuogee Hsieh 2022-06-24 21:40 ` Doug Anderson 2022-06-24 21:40 ` Doug Anderson 2022-06-24 21:50 ` Kuogee Hsieh 2022-06-24 21:50 ` Kuogee Hsieh 2022-06-24 23:45 ` Dmitry Baryshkov 2022-06-24 23:45 ` Dmitry Baryshkov 2022-06-24 23:41 ` Dmitry Baryshkov [this message] 2022-06-24 23:41 ` Dmitry Baryshkov 2022-06-24 17:15 ` [PATCH v1 2/3] drm/msm/dp: decoupling dp->id out of dp controller_id at scxxxx_dp_cfg table Kuogee Hsieh 2022-06-24 17:15 ` Kuogee Hsieh 2022-06-24 20:00 ` Stephen Boyd 2022-06-24 20:00 ` Stephen Boyd 2022-06-24 21:17 ` Kuogee Hsieh 2022-06-24 21:17 ` Kuogee Hsieh 2022-06-24 21:40 ` Stephen Boyd 2022-06-24 21:40 ` Stephen Boyd 2022-06-24 21:49 ` Kuogee Hsieh 2022-06-24 21:49 ` Kuogee Hsieh 2022-06-24 22:19 ` Stephen Boyd 2022-06-24 22:19 ` Stephen Boyd 2022-06-24 22:53 ` Kuogee Hsieh 2022-06-24 22:53 ` Kuogee Hsieh 2022-06-24 23:12 ` Stephen Boyd 2022-06-24 23:12 ` Stephen Boyd 2022-06-24 23:30 ` Kuogee Hsieh 2022-06-24 23:30 ` Kuogee Hsieh 2022-06-24 23:45 ` Stephen Boyd 2022-06-24 23:45 ` Stephen Boyd 2022-06-24 23:53 ` Dmitry Baryshkov 2022-06-24 23:53 ` Dmitry Baryshkov 2022-06-24 23:56 ` Kuogee Hsieh 2022-06-24 23:56 ` Kuogee Hsieh 2022-06-25 0:03 ` Abhinav Kumar 2022-06-25 0:03 ` Abhinav Kumar 2022-06-25 0:11 ` Stephen Boyd 2022-06-25 0:11 ` Stephen Boyd 2022-06-25 0:23 ` Stephen Boyd 2022-06-25 0:23 ` Stephen Boyd 2022-06-25 1:15 ` Abhinav Kumar 2022-06-25 1:15 ` Abhinav Kumar 2022-06-25 0:11 ` Dmitry Baryshkov 2022-06-25 0:11 ` Dmitry Baryshkov 2022-06-25 0:19 ` Kuogee Hsieh 2022-06-25 0:19 ` Kuogee Hsieh 2022-06-25 0:21 ` Dmitry Baryshkov 2022-06-25 0:21 ` Dmitry Baryshkov 2022-06-25 0:23 ` Kuogee Hsieh 2022-06-25 0:23 ` Kuogee Hsieh 2022-06-25 0:28 ` Dmitry Baryshkov 2022-06-25 0:28 ` Dmitry Baryshkov 2022-06-25 0:46 ` Dmitry Baryshkov 2022-06-25 0:46 ` Dmitry Baryshkov 2022-06-25 1:02 ` Kuogee Hsieh 2022-06-25 1:02 ` Kuogee Hsieh 2022-06-25 1:15 ` Stephen Boyd 2022-06-25 1:15 ` Stephen Boyd 2022-06-27 15:33 ` Kuogee Hsieh 2022-06-27 15:33 ` Kuogee Hsieh 2022-06-27 15:38 ` Dmitry Baryshkov 2022-06-27 15:38 ` Dmitry Baryshkov 2022-06-27 15:49 ` Kuogee Hsieh 2022-06-27 15:49 ` Kuogee Hsieh 2022-06-25 1:23 ` Abhinav Kumar 2022-06-25 1:23 ` Abhinav Kumar 2022-06-25 8:48 ` Dmitry Baryshkov 2022-06-25 8:48 ` Dmitry Baryshkov 2022-06-27 23:20 ` Doug Anderson 2022-06-27 23:20 ` Doug Anderson 2022-06-28 15:22 ` Kuogee Hsieh 2022-06-28 15:22 ` Kuogee Hsieh 2022-06-24 23:25 ` Dmitry Baryshkov 2022-06-24 23:25 ` Dmitry Baryshkov 2022-06-24 17:15 ` [PATCH v1 3/3] drm/msm/dp: place edp at head of drm bridge chain to fix screen corruption Kuogee Hsieh 2022-06-24 17:15 ` Kuogee Hsieh 2022-06-24 23:56 ` Dmitry Baryshkov 2022-06-24 23:56 ` Dmitry Baryshkov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAA8EJpozw5r657SaohnFzNzg59sO41+Q2qJkcjxDmCNCPnVe0w@mail.gmail.com \ --to=dmitry.baryshkov@linaro.org \ --cc=agross@kernel.org \ --cc=airlied@linux.ie \ --cc=bjorn.andersson@linaro.org \ --cc=daniel@ffwll.ch \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=freedreno@lists.freedesktop.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=quic_abhinavk@quicinc.com \ --cc=quic_aravindh@quicinc.com \ --cc=quic_khsieh@quicinc.com \ --cc=quic_sbillaka@quicinc.com \ --cc=robdclark@gmail.com \ --cc=sean@poorly.run \ --cc=swboyd@chromium.org \ --cc=vkoul@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.