Re: [PATCH 12/12] drm/msm: drop msm_kms_funcs::get_format() callback

[Abhinav Kumar]

On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:

Now as all subdrivers were converted to use common database of formats,
drop the get_format() callback and use mdp_get_format() directly.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c  | 2 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 1 -
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c| 2 +-
  drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 1 -
  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 1 -
  drivers/gpu/drm/msm/msm_fb.c | 2 +-
  drivers/gpu/drm/msm/msm_kms.h| 4 
  8 files changed, 4 insertions(+), 11 deletions(-)



Reviewed-by: Abhinav Kumar 

Re: [PATCH 11/12] drm/msm: merge dpu format database to MDP formats

[Abhinav Kumar]

On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:

Finally remove duplication between DPU and generic MDP code by merging
DPU format lists to the MDP format database.

Signed-off-by: Dmitry Baryshkov 
---
  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |   2 +-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   |   4 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c   | 602 --
  drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h   |  23 -
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  10 -
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |   2 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |   3 +-
  drivers/gpu/drm/msm/disp/mdp_format.c | 595 +++--
  drivers/gpu/drm/msm/disp/mdp_kms.h|   2 -
  drivers/gpu/drm/msm/msm_drv.h |  12 +
  10 files changed, 549 insertions(+), 706 deletions(-)



I cross-checked a few macros visually (not each one) and it LGTM in 
terms of just moving it from dpu_formats.c to mdp_format.c


Even in this change I had the same concern about whether to use MDP for 
dpu formats.


But I think even if we make it MSM_*** then we will have to keep them in 
some msm_** header and not mdp_format.c


So lets go ahead with the MDP naming which you have. If we see its not 
working out later on, please be open to a mass renaming that time.





index dea6d47854fe..e7651a0e878c 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -267,6 +267,16 @@ enum msm_format_flags {
  #define MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB 
BIT(MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB_BIT)
  #define MSM_FORMAT_FLAG_ALPHA_ENABLE  BIT(MSM_FORMAT_FLAG_ALPHA_ENABLE_BIT)
  
+/**

+ * DPU HW,Component order color map
+ */
+enum {
+   C0_G_Y = 0,
+   C1_B_Cb = 1,
+   C2_R_Cr = 2,
+   C3_ALPHA = 3
+};
+
  /**
   * struct msm_format: defines the format configuration
   * @pixel_format: format fourcc
@@ -305,6 +315,8 @@ struct msm_format {
(((X)->fetch_mode == MDP_FETCH_UBWC) && \
 ((X)->flags & MSM_FORMAT_FLAG_COMPRESSED))
  
+const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, uint64_t modifier);

+
  struct msm_pending_timer;
  
  int msm_atomic_init_pending_timer(struct msm_pending_timer *timer,


I am now thinking that do you think it makes sense to move all 
MDP_FORMAT macros to a new mdp_formats.h including the RGB/YUV bitfield 
macros (even though I already acked that change).


Instead of bloating msm_drv.h even more?

Re: [PATCH v5 11/18] drm/msm: generate headers on the fly

[Dmitry Baryshkov]

On Fri, 12 Apr 2024 at 19:15, Jon Hunter  wrote:
>
> Hi Dmitry,
>
> On 01/04/2024 03:42, Dmitry Baryshkov wrote:
> > Generate DRM/MSM headers on the fly during kernel build. This removes a
> > need to push register changes to Mesa with the following manual
> > synchronization step. Existing headers will be removed in the following
> > commits (split away to ease reviews).
> >
> > Signed-off-by: Dmitry Baryshkov 
> > ---
> >   drivers/gpu/drm/msm/.gitignore |  1 +
> >   drivers/gpu/drm/msm/Makefile   | 97 
> > +-
> >   2 files changed, 77 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/.gitignore b/drivers/gpu/drm/msm/.gitignore
> > new file mode 100644
> > index ..9ab870da897d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/msm/.gitignore
> > @@ -0,0 +1 @@
> > +generated/
> > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> > index 26ed4f443149..c861de58286c 100644
> > --- a/drivers/gpu/drm/msm/Makefile
> > +++ b/drivers/gpu/drm/msm/Makefile
> > @@ -1,10 +1,11 @@
> >   # SPDX-License-Identifier: GPL-2.0
> >   ccflags-y := -I $(srctree)/$(src)
> > +ccflags-y += -I $(obj)/generated
> >   ccflags-y += -I $(srctree)/$(src)/disp/dpu1
> >   ccflags-$(CONFIG_DRM_MSM_DSI) += -I $(srctree)/$(src)/dsi
> >   ccflags-$(CONFIG_DRM_MSM_DP) += -I $(srctree)/$(src)/dp
> >
> > -msm-y := \
> > +adreno-y := \
> >   adreno/adreno_device.o \
> >   adreno/adreno_gpu.o \
> >   adreno/a2xx_gpu.o \
> > @@ -18,7 +19,11 @@ msm-y := \
> >   adreno/a6xx_gmu.o \
> >   adreno/a6xx_hfi.o \
> >
> > -msm-$(CONFIG_DRM_MSM_HDMI) += \
> > +adreno-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
> > +
> > +adreno-$(CONFIG_DRM_MSM_GPU_STATE)   += adreno/a6xx_gpu_state.o
> > +
> > +msm-display-$(CONFIG_DRM_MSM_HDMI) += \
> >   hdmi/hdmi.o \
> >   hdmi/hdmi_audio.o \
> >   hdmi/hdmi_bridge.o \
> > @@ -31,7 +36,7 @@ msm-$(CONFIG_DRM_MSM_HDMI) += \
> >   hdmi/hdmi_phy_8x74.o \
> >   hdmi/hdmi_pll_8960.o \
> >
> > -msm-$(CONFIG_DRM_MSM_MDP4) += \
> > +msm-display-$(CONFIG_DRM_MSM_MDP4) += \
> >   disp/mdp4/mdp4_crtc.o \
> >   disp/mdp4/mdp4_dsi_encoder.o \
> >   disp/mdp4/mdp4_dtv_encoder.o \
> > @@ -42,7 +47,7 @@ msm-$(CONFIG_DRM_MSM_MDP4) += \
> >   disp/mdp4/mdp4_kms.o \
> >   disp/mdp4/mdp4_plane.o \
> >
> > -msm-$(CONFIG_DRM_MSM_MDP5) += \
> > +msm-display-$(CONFIG_DRM_MSM_MDP5) += \
> >   disp/mdp5/mdp5_cfg.o \
> >   disp/mdp5/mdp5_cmd_encoder.o \
> >   disp/mdp5/mdp5_ctl.o \
> > @@ -55,7 +60,7 @@ msm-$(CONFIG_DRM_MSM_MDP5) += \
> >   disp/mdp5/mdp5_plane.o \
> >   disp/mdp5/mdp5_smp.o \
> >
> > -msm-$(CONFIG_DRM_MSM_DPU) += \
> > +msm-display-$(CONFIG_DRM_MSM_DPU) += \
> >   disp/dpu1/dpu_core_perf.o \
> >   disp/dpu1/dpu_crtc.o \
> >   disp/dpu1/dpu_encoder.o \
> > @@ -85,14 +90,16 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> >   disp/dpu1/dpu_vbif.o \
> >   disp/dpu1/dpu_writeback.o
> >
> > -msm-$(CONFIG_DRM_MSM_MDSS) += \
> > +msm-display-$(CONFIG_DRM_MSM_MDSS) += \
> >   msm_mdss.o \
> >
> > -msm-y += \
> > +msm-display-y += \
> >   disp/mdp_format.o \
> >   disp/mdp_kms.o \
> >   disp/msm_disp_snapshot.o \
> >   disp/msm_disp_snapshot_util.o \
> > +
> > +msm-y += \
> >   msm_atomic.o \
> >   msm_atomic_tracepoints.o \
> >   msm_debugfs.o \
> > @@ -115,12 +122,12 @@ msm-y += \
> >   msm_submitqueue.o \
> >   msm_gpu_tracepoints.o \
> >
> > -msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
> > - dp/dp_debug.o
> > +msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
> >
> > -msm-$(CONFIG_DRM_MSM_GPU_STATE)  += adreno/a6xx_gpu_state.o
> > +msm-display-$(CONFIG_DEBUG_FS) += \
> > + dp/dp_debug.o
> >
> > -msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> > +msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> >   dp/dp_catalog.o \
> >   dp/dp_ctrl.o \
> >   dp/dp_display.o \
> > @@ -130,21 +137,69 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> >   dp/dp_audio.o \
> >   dp/dp_utils.o
> >
> > -msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
> > -
> > -msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
> > +msm-display-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
> >
> > -msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \
> > +msm-display-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \
> >   dsi/dsi_cfg.o \
> >   dsi/dsi_host.o \
> >   dsi/dsi_manager.o \
> >   dsi/phy/dsi_phy.o
> >
> > -msm-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += dsi/phy/dsi_phy_28nm.o
> > -msm-$(CONFIG_DRM_MSM_DSI_20NM_PHY) += dsi/phy/dsi_phy_20nm.o
> > -msm-$(CONFIG_DRM_MSM_DSI_28NM_8960_PHY) += dsi/phy/dsi_phy_28nm_8960.o
> > -msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/phy/dsi_phy_14nm.o
> > -msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/phy/dsi_phy_10nm.o
> > -msm-$(CONFIG_DRM_MSM_DSI_7NM_PHY) += dsi/phy/dsi_phy_7nm.o
> > +msm-display-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += 

[PATCH] drm/msm/a6xx: Avoid a nullptr dereference when speedbin setting fails

[Konrad Dybcio]

Calling a6xx_destroy() before adreno_gpu_init() leads to a null pointer
dereference on:

msm_gpu_cleanup() : platform_set_drvdata(gpu->pdev, NULL);

as gpu->pdev is only assigned in:

a6xx_gpu_init()
|_ adreno_gpu_init
|_ msm_gpu_init()

Instead of relying on handwavy null checks down the cleanup chain,
explicitly de-allocate the LLC data and free a6xx_gpu instead.

Fixes: 76efc2453d0e ("drm/msm/gpu: Fix crash during system suspend after 
unbind")
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 0674aca0f8a3..d10323f15d40 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -3058,7 +3058,8 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 
ret = a6xx_set_supported_hw(>dev, config->info);
if (ret) {
-   a6xx_destroy(&(a6xx_gpu->base.base));
+   a6xx_llc_slices_destroy(a6xx_gpu);
+   kfree(a6xx_gpu);
return ERR_PTR(ret);
}
 

---
base-commit: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
change-id: 20240412-topic-adreno_nullptr_supphw-10dbf5330044

Best regards,
-- 
Konrad Dybcio