[Freedreno] [PATCH v1 8/8] drm/msm/dpu: move SSPP debugfs support from plane to SSPP code
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Fri Dec 10 00:19:44 UTC 2021
On 10/12/2021 01:27, Abhinav Kumar wrote:
>
>
> On 12/9/2021 2:18 PM, Abhinav Kumar wrote:
>>
>>
>> On 12/1/2021 2:26 PM, Dmitry Baryshkov wrote:
>>> We are preparing to change DPU plane implementation. Move SSPP debugfs
>>> code from dpu_plane.c to dpu_hw_sspp.c, where it belongs.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 67 +++++++++++++++++
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 4 +
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 82 +++------------------
>>> 4 files changed, 84 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> index d77eb7da5daf..ae3cf2e4d7d9 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>>> @@ -8,6 +8,8 @@
>>> #include "dpu_hw_sspp.h"
>>> #include "dpu_kms.h"
>>> +#include <drm/drm_file.h>
>>> +
>>> #define DPU_FETCH_CONFIG_RESET_VALUE 0x00000087
>>> /* DPU_SSPP_SRC */
>>> @@ -686,6 +688,71 @@ static void _setup_layer_ops(struct dpu_hw_pipe *c,
>>> c->ops.setup_cdp = dpu_hw_sspp_setup_cdp;
>>> }
>>> +#ifdef CONFIG_DEBUG_FS
>>> +int _dpu_hw_sspp_init_debugfs(struct dpu_hw_pipe *hw_pipe, struct
>>> dpu_kms *kms, struct dentry *entry)
>>> +{
>>> + const struct dpu_sspp_cfg *cfg = hw_pipe->cap;
>>> + const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
>>> + struct dentry *debugfs_root;
>>> + char sspp_name[32];
>>> +
>>> + snprintf(sspp_name, sizeof(sspp_name), "%d", hw_pipe->idx);
>>> +
>>> + /* create overall sub-directory for the pipe */
>>> + debugfs_root =
>>> + debugfs_create_dir(sspp_name, entry);
>>
>>
>> I would like to take a different approach to this. Let me know what
>> you think.
>>
>> Let the directory names still be the drm plane names as someone who
>> would first get the DRM state and then try to lookup the register
>> values of that plane would not know where to look now.
>>
>> Inside the /sys/kernel/debug/***/plane-X/ directory you can expose an
>> extra entry which tells the sspp_index.
>>
>> This will also establish the plane to SSPP mapping.
>>
>> Now when planes go virtual in the future, this will be helpful even more
>> so that we can know the plane to SSPP mapping.
>
> OR i like rob's suggestion of implementing the atomic_print_state
> callback which will printout the drm plane to SSPP mapping along with
> this change so that when we look at DRM state, we also know the plane
> to SSPP mapping and look in the right SSPP's dir.
I'd add atomic_print_state(), it seems simpler (and more future-proof).
>>
>>
>>> +
>>> + /* don't error check these */
>>> + debugfs_create_xul("features", 0600,
>>> + debugfs_root, (unsigned long *)&hw_pipe->cap->features);
>>> +
>>> + /* add register dump support */
>>> + dpu_debugfs_create_regset32("src_blk", 0400,
>>> + debugfs_root,
>>> + sblk->src_blk.base + cfg->base,
>>> + sblk->src_blk.len,
>>> + kms);
>>> +
>>> + if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
>>> + cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
>>> + cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
>>> + cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
>>> + dpu_debugfs_create_regset32("scaler_blk", 0400,
>>> + debugfs_root,
>>> + sblk->scaler_blk.base + cfg->base,
>>> + sblk->scaler_blk.len,
>>> + kms);
>>> +
>>> + if (cfg->features & BIT(DPU_SSPP_CSC) ||
>>> + cfg->features & BIT(DPU_SSPP_CSC_10BIT))
>>> + dpu_debugfs_create_regset32("csc_blk", 0400,
>>> + debugfs_root,
>>> + sblk->csc_blk.base + cfg->base,
>>> + sblk->csc_blk.len,
>>> + kms);
>>> +
>>> + debugfs_create_u32("xin_id",
>>> + 0400,
>>> + debugfs_root,
>>> + (u32 *) &cfg->xin_id);
>>> + debugfs_create_u32("clk_ctrl",
>>> + 0400,
>>> + debugfs_root,
>>> + (u32 *) &cfg->clk_ctrl);
>>> + debugfs_create_x32("creq_vblank",
>>> + 0600,
>>> + debugfs_root,
>>> + (u32 *) &sblk->creq_vblank);
>>> + debugfs_create_x32("danger_vblank",
>>> + 0600,
>>> + debugfs_root,
>>> + (u32 *) &sblk->danger_vblank);
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> +
>>> static const struct dpu_sspp_cfg *_sspp_offset(enum dpu_sspp sspp,
>>> void __iomem *addr,
>>> struct dpu_mdss_cfg *catalog,
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> index e8939d7387cb..cef281687bab 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> @@ -381,6 +381,7 @@ struct dpu_hw_pipe {
>>> struct dpu_hw_sspp_ops ops;
>>> };
>>> +struct dpu_kms;
>>> /**
>>> * dpu_hw_sspp_init - initializes the sspp hw driver object.
>>> * Should be called once before accessing every pipe.
>>> @@ -400,5 +401,8 @@ struct dpu_hw_pipe *dpu_hw_sspp_init(enum
>>> dpu_sspp idx,
>>> */
>>> void dpu_hw_sspp_destroy(struct dpu_hw_pipe *ctx);
>>> +void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry
>>> *debugfs_root);
>>> +int _dpu_hw_sspp_init_debugfs(struct dpu_hw_pipe *hw_pipe, struct
>>> dpu_kms *kms, struct dentry *entry);
>>> +
>>> #endif /*_DPU_HW_SSPP_H */
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> index 7e7a619769a8..de9efe6dcf7c 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> @@ -281,6 +281,7 @@ static int dpu_kms_debugfs_init(struct msm_kms
>>> *kms, struct drm_minor *minor)
>>> dpu_debugfs_danger_init(dpu_kms, entry);
>>> dpu_debugfs_vbif_init(dpu_kms, entry);
>>> dpu_debugfs_core_irq_init(dpu_kms, entry);
>>> + dpu_debugfs_sspp_init(dpu_kms, entry);
>>> for (i = 0; i < ARRAY_SIZE(priv->dp); i++) {
>>> if (priv->dp[i])
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index ef66af696a40..cc7a7eb84fdd 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -13,7 +13,6 @@
>>> #include <drm/drm_atomic.h>
>>> #include <drm/drm_atomic_uapi.h>
>>> #include <drm/drm_damage_helper.h>
>>> -#include <drm/drm_file.h>
>>> #include <drm/drm_gem_atomic_helper.h>
>>> #include "msm_drv.h"
>>> @@ -1356,78 +1355,22 @@ void dpu_plane_danger_signal_ctrl(struct
>>> drm_plane *plane, bool enable)
>>> pm_runtime_put_sync(&dpu_kms->pdev->dev);
>>> }
>>> -static int _dpu_plane_init_debugfs(struct drm_plane *plane)
>>> +/* SSPP live inside dpu_plane private data only. Enumerate them
>>> here. */
>>> +void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry
>>> *debugfs_root)
>>> {
>>> - struct dpu_plane *pdpu = to_dpu_plane(plane);
>>> - struct dpu_kms *kms = _dpu_plane_get_kms(plane);
>>> - const struct dpu_sspp_cfg *cfg = pdpu->pipe_hw->cap;
>>> - const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
>>> - struct dentry *debugfs_root;
>>> -
>>> - /* create overall sub-directory for the pipe */
>>> - debugfs_root =
>>> - debugfs_create_dir(plane->name,
>>> - plane->dev->primary->debugfs_root);
>>> -
>>> - /* don't error check these */
>>> - debugfs_create_xul("features", 0600,
>>> - debugfs_root, (unsigned long
>>> *)&pdpu->pipe_hw->cap->features);
>>> -
>>> - /* add register dump support */
>>> - dpu_debugfs_create_regset32("src_blk", 0400,
>>> - debugfs_root,
>>> - sblk->src_blk.base + cfg->base,
>>> - sblk->src_blk.len,
>>> - kms);
>>> -
>>> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
>>> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
>>> - dpu_debugfs_create_regset32("scaler_blk", 0400,
>>> - debugfs_root,
>>> - sblk->scaler_blk.base + cfg->base,
>>> - sblk->scaler_blk.len,
>>> - kms);
>>> -
>>> - if (cfg->features & BIT(DPU_SSPP_CSC) ||
>>> - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
>>> - dpu_debugfs_create_regset32("csc_blk", 0400,
>>> - debugfs_root,
>>> - sblk->csc_blk.base + cfg->base,
>>> - sblk->csc_blk.len,
>>> - kms);
>>> -
>>> - debugfs_create_u32("xin_id",
>>> - 0400,
>>> - debugfs_root,
>>> - (u32 *) &cfg->xin_id);
>>> - debugfs_create_u32("clk_ctrl",
>>> - 0400,
>>> - debugfs_root,
>>> - (u32 *) &cfg->clk_ctrl);
>>> - debugfs_create_x32("creq_vblank",
>>> - 0600,
>>> - debugfs_root,
>>> - (u32 *) &sblk->creq_vblank);
>>> - debugfs_create_x32("danger_vblank",
>>> - 0600,
>>> - debugfs_root,
>>> - (u32 *) &sblk->danger_vblank);
>>> + struct drm_plane *plane;
>>> + struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
>>> - return 0;
>>> -}
>>> -#else
>>> -static int _dpu_plane_init_debugfs(struct drm_plane *plane)
>>> -{
>>> - return 0;
>>> -}
>>> -#endif
>>> + if (IS_ERR(entry))
>>> + return;
>>> -static int dpu_plane_late_register(struct drm_plane *plane)
>>> -{
>>> - return _dpu_plane_init_debugfs(plane);
>>> + drm_for_each_plane(plane, dpu_kms->dev) {
>>> + struct dpu_plane *pdpu = to_dpu_plane(plane);
>>> +
>>> + _dpu_hw_sspp_init_debugfs(pdpu->pipe_hw, dpu_kms, entry);
>>> + }
>>> }
>>> +#endif
>>> static bool dpu_plane_format_mod_supported(struct drm_plane *plane,
>>> uint32_t format, uint64_t modifier)
>>> @@ -1453,7 +1396,6 @@ static const struct drm_plane_funcs
>>> dpu_plane_funcs = {
>>> .reset = dpu_plane_reset,
>>> .atomic_duplicate_state = dpu_plane_duplicate_state,
>>> .atomic_destroy_state = dpu_plane_destroy_state,
>>> - .late_register = dpu_plane_late_register,
>>> .format_mod_supported = dpu_plane_format_mod_supported,
>>> };
>>>
--
With best wishes
Dmitry
More information about the dri-devel
mailing list