Re: [Freedreno] [PATCH] drm: remove drm_bridge_hpd_disable() from drm_bridge_connector_destroy()
On 18/07/2023 01:34, Abhinav Kumar wrote:
drm_bridge_hpd_enable()/drm_bridge_hpd_disable() callbacks call into
the respective driver's hpd_enable()/hpd_disable() ops. These ops control
the HPD enable/disable logic which in some cases like MSM can be a
dedicate hardware block to control the HPD.
During probe_defer cases, a connector can be initialized and then later
destroyed till the probe is retried. During connector destroy in these
cases, the hpd_disable() callback gets called without a corresponding
hpd_enable() leading to an unbalanced state potentially causing even
a crash.
This can be avoided by the respective drivers maintaining their own
state logic to ensure that a hpd_disable() without a corresponding
hpd_enable() just returns without doing anything.
However, to have a generic fix it would be better to avoid the
hpd_disable() callback from the connector destroy path and let
the hpd_enable() / hpd_disable() balance be maintained by the
corresponding drm_bridge_connector_enable_hpd() /
drm_bridge_connector_disable_hpd() APIs.
Which should get called by the drm_kms_helper_disable_hpd().
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/drm_bridge_connector.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 19ae4a177ac3..3d4e25c2f3d7 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -187,12 +187,6 @@ static void drm_bridge_connector_destroy(struct
drm_connector *connector)
struct drm_bridge_connector *bridge_connector =
to_drm_bridge_connector(connector);
- if (bridge_connector->bridge_hpd) {
- struct drm_bridge *hpd = bridge_connector->bridge_hpd;
-
- drm_bridge_hpd_disable(hpd);
- }
-
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH 0/5] arm64: dts: qcom: qrb5165-rb5: enable DP support
On Sun, Jul 09, 2023 at 07:19:21AM +0300, Dmitry Baryshkov wrote: > Implement DisplayPort support for the Qualcomm RB5 platform. > > Note: while testing this, I had link training issues with several > dongles with DP connectors. Other DisplayPort-USB-C dongles (with HDMI > or VGA connectors) work perfectly. > > Dependencies: [1] > Soft-dependencies: [2], [3] > > [1] > https://lore.kernel.org/linux-arm-msm/20230515133643.3621656-1-bryan.odonog...@linaro.org/ I'm not able to find a version of this series ready to be merged, can you please help me find it? Regards, Bjorn
Re: [Freedreno] [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/14/2023 6:38 PM, Dmitry Baryshkov wrote:
On 15/07/2023 03:59, Jessica Zhang wrote:
On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote:
On Fri, 14 Jul 2023 at 22:03, Jessica Zhang
wrote:
On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote:
On 14/07/2023 03:21, Jessica Zhang wrote:
DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that
allows DSI
to send 48 bits of compressed data per pclk instead of 24.
For all chipsets that support this mode, enable it whenever DSC is
enabled as recommended by the hardware programming guide.
Only enable this for command mode as we are currently unable to
validate
it for video mode.
Signed-off-by: Jessica Zhang
---
Note: The dsi.xml.h changes were generated using the headergen2
script in
envytools [2], but the changes to the copyright and rules-ng-ng
source
file
paths were dropped.
Separate commit please, so that it can be replaced by headers sync
with
Mesa3d.
Hi Dmitry,
Acked.
[1] https://patchwork.freedesktop.org/series/120580/
[2] https://github.com/freedreno/envytools/
--
Changes in v2:
- Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision"
- Squashed all commits to avoid breaking feature if the series is
only
partially applied
No. Please unsquash it. Please design the series so that the patches
work even if it is only partially applied.
Acked.
- Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn)
- Have DPU check if wide bus is requested by output driver (Dmitry)
- Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay
adjustment (Marijn)
- Link to v1:
https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 +
drivers/gpu/drm/msm/dsi/dsi.c | 5 +
drivers/gpu/drm/msm/dsi/dsi.h | 1 +
drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 23
+-
drivers/gpu/drm/msm/msm_drv.h | 6 ++
9 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f0a2a1dca741..6aed63c06c1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct
drm_device *dev,
struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
struct drm_encoder *drm_enc = NULL;
struct dpu_encoder_virt *dpu_enc = NULL;
+ int index = disp_info->h_tile_instance[0];
int ret = 0;
dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc),
GFP_KERNEL);
@@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct
drm_device *dev,
timer_setup(_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0);
- if (disp_info->intf_type == INTF_DSI)
+ if (disp_info->intf_type == INTF_DSI) {
timer_setup(_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
While you are touching this part, could you please drop
dpu_encoder_vsync_event_handler() and
dpu_encoder_vsync_event_work_handler(), they are useless?
Since these calls aren't related to widebus, I don't think I'll include
it in this series. However, I can post this cleanup as a separate patch
and add that as a dependency if that's ok.
Sure, that will work for me. Thank you!
0);
- else if (disp_info->intf_type == INTF_DP)
- dpu_enc->wide_bus_en = msm_dp_wide_bus_available(
- priv->dp[disp_info->h_tile_instance[0]]);
+ dpu_enc->wide_bus_en =
msm_dsi_is_widebus_enabled(priv->dsi[index]);
+ } else if (disp_info->intf_type == INTF_DP) {
+ dpu_enc->wide_bus_en =
msm_dp_wide_bus_available(priv->dp[index]);
+ }
INIT_DELAYED_WORK(_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index df88358e7037..dace6168be2d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -69,8 +69,10 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
phys_enc->hw_intf,
phys_enc->hw_pp->idx);
- if (intf_cfg.dsc != 0)
+ if (intf_cfg.dsc != 0) {
cmd_mode_cfg.data_compress = true;
+ cmd_mode_cfg.wide_bus_en =
dpu_encoder_is_widebus_enabled(phys_enc->parent);
+ }
if (phys_enc->hw_intf->ops.program_intf_cmd_cfg)
phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf,
_mode_cfg);
diff --git
Re: [Freedreno] (subset) [PATCH v2 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel
On Tue, 27 Jun 2023 22:14:15 +0200, Marijn Suijten wrote: > Bring up the SM6125 DPU now that all preliminary series (such as INTF > TE) have been merged (for me to test the hardware properly), and most > other conflicting work (barring ongoing catalog *improvements*) has made > its way in as well or is still being discussed. > > The second part of the series complements that by immediately utilizing > this hardware in DT, and even enabling the MDSS/DSI nodes complete with > a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II). > > [...] Applied, thanks! [01/15] drm/msm/dsi: Drop unused regulators from QCM2290 14nm DSI PHY config https://gitlab.freedesktop.org/drm/msm/-/commit/97368254a08e Best regards, -- Abhinav Kumar
Re: [Freedreno] (subset) [PATCH v3 00/11] drm/msm/dpu: cleanup dpu_core_perf module
On Fri, 07 Jul 2023 22:39:31 +0300, Dmitry Baryshkov wrote: > Apply several cleanups to the DPU's core_perf module. > > Changes since v2: > - Dropped perf tuning patches for now (Abhinav) > - Restored kms variable assignment in dpu_core_perf_crtc_release_bw > (LKP) > - Fixed description for the last patch (Abhinav) > > [...] Applied, thanks! [01/11] drm/msm/dpu: drop enum dpu_core_perf_data_bus_id https://gitlab.freedesktop.org/drm/msm/-/commit/e8383f5cf1b3 Best regards, -- Abhinav Kumar
Re: [Freedreno] [PATCH] drm/msm/dpu: add missing flush and fetch bits for DMA4/DMA5 planes
On Tue, 04 Jul 2023 12:01:04 -0400, Jonathan Marek wrote: > Note that with this, DMA4/DMA5 are still non-functional, but at least > display *something* in modetest instead of nothing or underflow. > > Applied, thanks! [1/1] drm/msm/dpu: add missing flush and fetch bits for DMA4/DMA5 planes https://gitlab.freedesktop.org/drm/msm/-/commit/ba7a94ea7312 Best regards, -- Abhinav Kumar
Re: [Freedreno] (subset) [PATCH v2 0/8] MDSS reg bus interconnect
On Wed, 12 Jul 2023 15:11:37 +0300, Dmitry Baryshkov wrote: > Per agreement with Konrad, picked up this patch series. > > Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's > another path that needs to be handled to ensure MDSS functions properly, > namely the "reg bus", a.k.a the CPU-MDSS interconnect. > > Gating that path may have a variety of effects. from none to otherwise > inexplicable DSI timeouts. > > [...] Applied, thanks! [2/8] drm/msm/mdss: correct UBWC programming for SM8550 https://gitlab.freedesktop.org/drm/msm/-/commit/a85c238c5ccd Best regards, -- Abhinav Kumar
[Freedreno] [PATCH] drm: remove drm_bridge_hpd_disable() from drm_bridge_connector_destroy()
drm_bridge_hpd_enable()/drm_bridge_hpd_disable() callbacks call into
the respective driver's hpd_enable()/hpd_disable() ops. These ops control
the HPD enable/disable logic which in some cases like MSM can be a
dedicate hardware block to control the HPD.
During probe_defer cases, a connector can be initialized and then later
destroyed till the probe is retried. During connector destroy in these
cases, the hpd_disable() callback gets called without a corresponding
hpd_enable() leading to an unbalanced state potentially causing even
a crash.
This can be avoided by the respective drivers maintaining their own
state logic to ensure that a hpd_disable() without a corresponding
hpd_enable() just returns without doing anything.
However, to have a generic fix it would be better to avoid the
hpd_disable() callback from the connector destroy path and let
the hpd_enable() / hpd_disable() balance be maintained by the
corresponding drm_bridge_connector_enable_hpd() /
drm_bridge_connector_disable_hpd() APIs.
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/drm_bridge_connector.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/drm_bridge_connector.c
index 19ae4a177ac3..3d4e25c2f3d7 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -187,12 +187,6 @@ static void drm_bridge_connector_destroy(struct
drm_connector *connector)
struct drm_bridge_connector *bridge_connector =
to_drm_bridge_connector(connector);
- if (bridge_connector->bridge_hpd) {
- struct drm_bridge *hpd = bridge_connector->bridge_hpd;
-
- drm_bridge_hpd_disable(hpd);
- }
-
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
--
2.40.1
Re: [Freedreno] [PATCH 12/12] drm/msm/adreno: Switch to chip-id for identifying GPU
On Thu, Jul 13, 2023 at 03:06:36PM -0700, Rob Clark wrote: > > On Thu, Jul 13, 2023 at 2:39 PM Akhil P Oommen > wrote: > > > > On Fri, Jul 07, 2023 at 06:45:42AM +0300, Dmitry Baryshkov wrote: > > > > > > On 07/07/2023 00:10, Rob Clark wrote: > > > > From: Rob Clark > > > > > > > > Since the revision becomes an opaque identifier with future GPUs, move > > > > away from treating different ranges of bits as having a given meaning. > > > > This means that we need to explicitly list different patch revisions in > > > > the device table. > > > > > > > > Signed-off-by: Rob Clark > > > > --- > > > > drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 2 +- > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 11 +- > > > > drivers/gpu/drm/msm/adreno/a5xx_power.c| 2 +- > > > > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 13 ++- > > > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +- > > > > drivers/gpu/drm/msm/adreno/adreno_device.c | 128 ++--- > > > > drivers/gpu/drm/msm/adreno/adreno_gpu.c| 16 +-- > > > > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 51 > > > > 8 files changed, 122 insertions(+), 110 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > > > > b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > > > > index 715436cb3996..8b4cdf95f445 100644 > > > > --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > > > > +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > > > > @@ -145,7 +145,7 @@ static void a4xx_enable_hwcg(struct msm_gpu *gpu) > > > > gpu_write(gpu, REG_A4XX_RBBM_CLOCK_DELAY_HLSQ, 0x0022); > > > > /* Early A430's have a timing issue with SP/TP power collapse; > > > >disabling HW clock gating prevents it. */ > > > > - if (adreno_is_a430(adreno_gpu) && adreno_gpu->rev.patchid < 2) > > > > + if (adreno_is_a430(adreno_gpu) && adreno_patchid(adreno_gpu) < 2) > > > > gpu_write(gpu, REG_A4XX_RBBM_CLOCK_CTL, 0); > > > > else > > > > gpu_write(gpu, REG_A4XX_RBBM_CLOCK_CTL, 0x); > > > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > > b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > > index f0803e94ebe5..70d2b5342cd9 100644 > > > > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > > > > @@ -1744,6 +1744,7 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device > > > > *dev) > > > > struct msm_drm_private *priv = dev->dev_private; > > > > struct platform_device *pdev = priv->gpu_pdev; > > > > struct adreno_platform_config *config = pdev->dev.platform_data; > > > > + const struct adreno_info *info; > > > > struct a5xx_gpu *a5xx_gpu = NULL; > > > > struct adreno_gpu *adreno_gpu; > > > > struct msm_gpu *gpu; > > > > @@ -1770,7 +1771,15 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device > > > > *dev) > > > > nr_rings = 4; > > > > - if (adreno_cmp_rev(ADRENO_REV(5, 1, 0, ANY_ID), config->rev)) > > > > + /* > > > > +* Note that we wouldn't have been able to get this far if there is > > > > not > > > > +* a device table entry for this chip_id > > > > +*/ > > > > + info = adreno_find_info(config->chip_id); > > > > + if (WARN_ON(!info)) > > > > + return ERR_PTR(-EINVAL); > > > > + > > > > + if (info->revn == 510) > > > > nr_rings = 1; > > > > ret = adreno_gpu_init(dev, pdev, adreno_gpu, , nr_rings); > > > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c > > > > b/drivers/gpu/drm/msm/adreno/a5xx_power.c > > > > index 0e63a1429189..7705f8010484 100644 > > > > --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c > > > > +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c > > > > @@ -179,7 +179,7 @@ static void a540_lm_setup(struct msm_gpu *gpu) > > > > /* The battery current limiter isn't enabled for A540 */ > > > > config = AGC_LM_CONFIG_BCL_DISABLED; > > > > - config |= adreno_gpu->rev.patchid << > > > > AGC_LM_CONFIG_GPU_VERSION_SHIFT; > > > > + config |= adreno_patchid(adreno_gpu) << > > > > AGC_LM_CONFIG_GPU_VERSION_SHIFT; > > > > /* For now disable GPMU side throttling */ > > > > config |= AGC_LM_CONFIG_THROTTLE_DISABLE; > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > > > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > > > index f1bb20574018..a9ba547a120c 100644 > > > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > > > @@ -790,10 +790,15 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu > > > > *gmu, unsigned int state) > > > > gmu_write(gmu, REG_A6XX_GMU_AHB_FENCE_RANGE_0, > > > > (1 << 31) | (0xa << 18) | (0xa0)); > > > > - chipid = adreno_gpu->rev.core << 24; > > > > - chipid |= adreno_gpu->rev.major << 16; > > > > - chipid |= adreno_gpu->rev.minor << 12; > > > > - chipid |= adreno_gpu->rev.patchid << 8; > > > > + /* Note that the GMU has a slightly different layout for > > > > +* chip_id, for whatever reason, so a bit of massaging > > > > +
Re: [Freedreno] [PATCH 05/12] drm/msm/adreno: Use quirk to identify cached-coherent support
On Thu, Jul 13, 2023 at 03:25:33PM -0700, Rob Clark wrote:
>
> On Thu, Jul 13, 2023 at 1:06 PM Akhil P Oommen
> wrote:
> >
> > On Thu, Jul 06, 2023 at 02:10:38PM -0700, Rob Clark wrote:
> > >
> > > From: Rob Clark
> > >
> > > It is better to explicitly list it. With the move to opaque chip-id's
> > > for future devices, we should avoid trying to infer things like
> > > generation from the numerical value.
> > >
> > > Signed-off-by: Rob Clark
> > > ---
> > > drivers/gpu/drm/msm/adreno/adreno_device.c | 23 +++---
> > > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 1 +
> > > 2 files changed, 17 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > index f469f951a907..3c531da417b9 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -256,6 +256,7 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_512K,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > > .init = a6xx_gpu_init,
> > > }, {
> > > .rev = ADRENO_REV(6, 1, 9, ANY_ID),
> > > @@ -266,6 +267,7 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_512K,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a615_zap.mdt",
> > > .hwcg = a615_hwcg,
> > > @@ -278,6 +280,7 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_1M,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a630_zap.mdt",
> > > .hwcg = a630_hwcg,
> > > @@ -290,6 +293,7 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_1M,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a640_zap.mdt",
> > > .hwcg = a640_hwcg,
> > > @@ -302,7 +306,8 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_1M + SZ_128K,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > - .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > + ADRENO_QUIRK_HAS_HW_APRIV,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a650_zap.mdt",
> > > .hwcg = a650_hwcg,
> > > @@ -316,7 +321,8 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_1M + SZ_512K,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > - .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > + ADRENO_QUIRK_HAS_HW_APRIV,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a660_zap.mdt",
> > > .hwcg = a660_hwcg,
> > > @@ -329,7 +335,8 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_512K,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > - .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > + ADRENO_QUIRK_HAS_HW_APRIV,
> > > .init = a6xx_gpu_init,
> > > .hwcg = a660_hwcg,
> > > .address_space_size = SZ_16G,
> > > @@ -342,6 +349,7 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_2M,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a640_zap.mdt",
> > > .hwcg = a640_hwcg,
> > > @@ -353,7 +361,8 @@ static const struct adreno_info gpulist[] = {
> > > },
> > > .gmem = SZ_4M,
> > > .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > - .quirks = ADRENO_QUIRK_HAS_HW_APRIV,
> > > + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT |
> > > + ADRENO_QUIRK_HAS_HW_APRIV,
> > > .init = a6xx_gpu_init,
> > > .zapfw = "a690_zap.mdt",
> > > .hwcg = a690_hwcg,
> > > @@ -565,9 +574,9 @@ static int adreno_bind(struct device *dev, struct
> > > device *master, void *data)
> > > if (ret)
Re: [Freedreno] [PATCH v1 2/5] drm/msm/dp: incorporate pm_runtime framework into DP driver
On 7/7/2023 5:04 PM, Dmitry Baryshkov wrote:
On 08/07/2023 02:52, Kuogee Hsieh wrote:
Incorporating pm runtime framework into DP driver so that power
and clock resource handling can be centralized allowing easier
control of these resources in preparation of registering aux bus
uring probe.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_aux.c | 3 ++
drivers/gpu/drm/msm/dp/dp_display.c | 75
+
2 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c
b/drivers/gpu/drm/msm/dp/dp_aux.c
index 8e3b677..c592064 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -291,6 +291,7 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux
*dp_aux,
return -EINVAL;
}
+ pm_runtime_get_sync(dp_aux->dev);
Let me quote the function's documentation:
Consider using pm_runtime_resume_and_get() instead of it, especially
if its return value is checked by the caller, as this is likely to
result in cleaner code.
So two notes concerning the whole patch:
- error checking is missing
- please use pm_runtime_resume_and_get() instead.
mutex_lock(>mutex);
if (!aux->initted) {
ret = -EIO;
@@ -364,6 +365,8 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux
*dp_aux,
exit:
mutex_unlock(>mutex);
+ pm_runtime_mark_last_busy(dp_aux->dev);
+ pm_runtime_put_autosuspend(dp_aux->dev);
return ret;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 76f1395..2c5706a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -309,6 +309,10 @@ static int dp_display_bind(struct device *dev,
struct device *master,
goto end;
}
+ pm_runtime_enable(dev);
devm_pm_runtime_enable() removes need for a cleanup.
+ pm_runtime_set_autosuspend_delay(dev, 1000);
+ pm_runtime_use_autosuspend(dev);
Why do you want to use autosuspend here?
+
return 0;
end:
return rc;
@@ -320,9 +324,8 @@ static void dp_display_unbind(struct device *dev,
struct device *master,
struct dp_display_private *dp = dev_get_dp_display_private(dev);
struct msm_drm_private *priv = dev_get_drvdata(master);
- /* disable all HPD interrupts */
- if (dp->core_initialized)
- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK,
false);
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
kthread_stop(dp->ev_tsk);
@@ -466,10 +469,12 @@ static void dp_display_host_init(struct
dp_display_private *dp)
dp->dp_display.connector_type, dp->core_initialized,
dp->phy_initialized);
- dp_power_init(dp->power);
- dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
- dp_aux_init(dp->aux);
- dp->core_initialized = true;
+ if (!dp->core_initialized) {
+ dp_power_init(dp->power);
+ dp_ctrl_reset_irq_ctrl(dp->ctrl, true);
+ dp_aux_init(dp->aux);
+ dp->core_initialized = true;
+ }
Is this relevant to PM runtime? I don't think so.
}
static void dp_display_host_deinit(struct dp_display_private *dp)
@@ -478,10 +483,12 @@ static void dp_display_host_deinit(struct
dp_display_private *dp)
dp->dp_display.connector_type, dp->core_initialized,
dp->phy_initialized);
- dp_ctrl_reset_irq_ctrl(dp->ctrl, false);
- dp_aux_deinit(dp->aux);
- dp_power_deinit(dp->power);
- dp->core_initialized = false;
+ if (dp->core_initialized) {
+ dp_ctrl_reset_irq_ctrl(dp->ctrl, false);
+ dp_aux_deinit(dp->aux);
+ dp_power_deinit(dp->power);
+ dp->core_initialized = false;
+ }
}
static int dp_display_usbpd_configure_cb(struct device *dev)
@@ -1304,6 +1311,39 @@ static int dp_display_remove(struct
platform_device *pdev)
dp_display_deinit_sub_modules(dp);
platform_set_drvdata(pdev, NULL);
+ pm_runtime_put_sync_suspend(>dev);
+
+ return 0;
+}
+
+static int dp_pm_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct msm_dp *dp_display = platform_get_drvdata(pdev);
+ struct dp_display_private *dp;
+
+ dp = container_of(dp_display, struct dp_display_private,
dp_display);
+
+ dp_display_host_phy_exit(dp);
+ dp_catalog_ctrl_hpd_enable(dp->catalog);
What? NO!
+ dp_display_host_deinit(dp);
+
+ return 0;
+}
+
+static int dp_pm_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct msm_dp *dp_display = platform_get_drvdata(pdev);
+ struct dp_display_private *dp;
+
+ dp = container_of(dp_display, struct dp_display_private,
dp_display);
+
+ dp_display_host_init(dp);
+ if (dp_display->is_edp) {
+ dp_catalog_ctrl_hpd_enable(dp->catalog);
+ dp_display_host_phy_init(dp);
+ }
return 0;
}
@@ -1409,6 +1449,7 @@ static int
Re: [Freedreno] [PATCH v1 4/5] drm/msm/dp: move relevant dp initialization code from bind() to probe()
On 7/17/2023 10:22 AM, Dmitry Baryshkov wrote:
On 17/07/2023 20:16, Kuogee Hsieh wrote:
On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
On 10/07/2023 19:57, Kuogee Hsieh wrote:
On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
On 08/07/2023 02:52, Kuogee Hsieh wrote:
In preparation of moving edp of_dp_aux_populate_bus() to
dp_display_probe(), move dp_display_request_irq(),
dp->parser->parse() and dp_power_client_init() to dp_display_probe()
too.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 48
+
drivers/gpu/drm/msm/dp/dp_display.h | 1 -
2 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 44580c2..185f1eb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -290,12 +290,6 @@ static int dp_display_bind(struct device
*dev, struct device *master,
goto end;
}
- rc = dp_power_client_init(dp->power);
- if (rc) {
- DRM_ERROR("Power client create failed\n");
- goto end;
- }
-
rc = dp_register_audio_driver(dev, dp->audio);
if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
@@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp->parser->parse(dp->parser);
+ if (rc) {
+ DRM_ERROR("device tree parsing failed\n");
+ goto error;
+ }
+
dp->catalog = dp_catalog_get(dev, >parser->io);
if (IS_ERR(dp->catalog)) {
rc = PTR_ERR(dp->catalog);
@@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp_power_client_init(dp->power);
+ if (rc) {
+ DRM_ERROR("Power client create failed\n");
+ goto error;
+ }
+
dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
if (IS_ERR(dp->aux)) {
rc = PTR_ERR(dp->aux);
@@ -1196,26 +1202,20 @@ static irqreturn_t
dp_display_irq_handler(int irq, void *dev_id)
return ret;
}
-int dp_display_request_irq(struct msm_dp *dp_display)
+static int dp_display_request_irq(struct dp_display_private *dp)
{
int rc = 0;
- struct dp_display_private *dp;
-
- if (!dp_display) {
- DRM_ERROR("invalid input\n");
- return -EINVAL;
- }
-
- dp = container_of(dp_display, struct dp_display_private,
dp_display);
+ struct device *dev = >pdev->dev;
- dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
if (!dp->irq) {
- DRM_ERROR("failed to get irq\n");
- return -EINVAL;
+ dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
+ if (!dp->irq) {
+ DRM_ERROR("failed to get irq\n");
+ return -EINVAL;
+ }
}
Use platform_get_irq() from probe() function.
- rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
- dp_display_irq_handler,
+ rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
if (rc < 0) {
DRM_ERROR("failed to request IRQ%u: %d\n",
@@ -1290,6 +1290,8 @@ static int dp_display_probe(struct
platform_device *pdev)
platform_set_drvdata(pdev, >dp_display);
+ dp_display_request_irq(dp);
+
Error checking?
Are we completely ready to handle interrupts at this point?
not until dp_display_host_init() is called which will be called
from pm_runtime_resume() later.
But once you request_irq(), you should be ready for the IRQs to be
delivered right away.
At this point, the DP controller interrupts mask bit is not enabled yet.
Therefore interrupts will not happen until dp_bridge_hpd_enable() is
called to initialize dp host controller and then enabled mask bits.
Are AUX and CTRL interrupts also disabled? What about any
stray/pending interrupts? Just take it as a rule of thumb. Once
request_irq() has been called without the IRQ_NOAUTOEN flag, the
driver should be prepared to handle the incoming interrupt requests.
yes, both AUX and CTRL are disabled.
edp population do need irq to handle aux transfer during probe.
it should work by checking core_initialized flag at irq handle to filter
out stray/pending interrupts.
rc = component_add(>dev, _display_comp_ops);
if (rc) {
DRM_ERROR("component add failed, rc=%d\n", rc);
@@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp
*dp_display, struct drm_device *dev,
dp_priv = container_of(dp_display, struct
dp_display_private, dp_display);
- ret = dp_display_request_irq(dp_display);
- if (ret) {
- DRM_ERROR("request_irq failed, ret=%d\n", ret);
- return ret;
- }
-
ret = dp_display_get_next_bridge(dp_display);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index
Re: [Freedreno] [PATCH v4 02/11] drm/msm/dpu: core_perf: extract bandwidth aggregation function
On Mon, 17 Jul 2023 at 21:45, Abhinav Kumar wrote:
>
>
>
> On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote:
> > In preparation to refactoring the dpu_core_perf debugfs interface,
> > extract the bandwidth aggregation function from
> > _dpu_core_perf_crtc_update_bus().
> >
> > Signed-off-by: Dmitry Baryshkov
> > ---
>
> Drop the core_perf tag from the subject line.
Ack.
>
> The debugfs refactor was dropped from this series if thats what you are
> referring to here.
>
> So even this and the next patch dont serve any purpose in this series
> and should be dropped, Unless you have some reason of keeping them here.
The next patch serves its purpose: it prevents recalculating bandwidth
if there are no ICC paths (useless math).
This patch separates actual math and control paths. It was required
for debugfs, but it serves its own purpose too. I'll reflect that in
the commit message.
>
> > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 39 +++
> > 1 file changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> > index 1d9d83d7b99e..333dcfe57800 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> > @@ -206,33 +206,38 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
> > return 0;
> > }
> >
> > -static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
> > - struct drm_crtc *crtc)
> > +static void dpu_core_perf_aggregate(struct drm_device *ddev,
> > + enum dpu_crtc_client_type
> > curr_client_type,
> > + struct dpu_core_perf_params *perf)
> > {
> > - struct dpu_core_perf_params perf = { 0 };
> > - enum dpu_crtc_client_type curr_client_type
> > - = dpu_crtc_get_client_type(crtc);
> > - struct drm_crtc *tmp_crtc;
> > struct dpu_crtc_state *dpu_cstate;
> > - int i, ret = 0;
> > - u64 avg_bw;
> > + struct drm_crtc *tmp_crtc;
> >
> > - drm_for_each_crtc(tmp_crtc, crtc->dev) {
> > + drm_for_each_crtc(tmp_crtc, ddev) {
> > if (tmp_crtc->enabled &&
> > - curr_client_type ==
> > - dpu_crtc_get_client_type(tmp_crtc)) {
> > + curr_client_type == dpu_crtc_get_client_type(tmp_crtc)) {
> > dpu_cstate = to_dpu_crtc_state(tmp_crtc->state);
> >
> > - perf.max_per_pipe_ib = max(perf.max_per_pipe_ib,
> > - dpu_cstate->new_perf.max_per_pipe_ib);
> > + perf->max_per_pipe_ib = max(perf->max_per_pipe_ib,
> > +
> > dpu_cstate->new_perf.max_per_pipe_ib);
> >
> > - perf.bw_ctl += dpu_cstate->new_perf.bw_ctl;
> > + perf->bw_ctl += dpu_cstate->new_perf.bw_ctl;
> >
> > - DRM_DEBUG_ATOMIC("crtc=%d bw=%llu paths:%d\n",
> > - tmp_crtc->base.id,
> > - dpu_cstate->new_perf.bw_ctl,
> > kms->num_paths);
> > + DRM_DEBUG_ATOMIC("crtc=%d bw=%llu\n",
> > + tmp_crtc->base.id,
> > + dpu_cstate->new_perf.bw_ctl);
> > }
> > }
> > +}
> > +
> > +static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
> > + struct drm_crtc *crtc)
> > +{
> > + struct dpu_core_perf_params perf = { 0 };
> > + int i, ret = 0;
> > + u64 avg_bw;
> > +
> > + dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc),
> > );
> >
> > if (!kms->num_paths)
> > return 0;
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v4 02/11] drm/msm/dpu: core_perf: extract bandwidth aggregation function
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote:
In preparation to refactoring the dpu_core_perf debugfs interface,
extract the bandwidth aggregation function from
_dpu_core_perf_crtc_update_bus().
Signed-off-by: Dmitry Baryshkov
---
Drop the core_perf tag from the subject line.
The debugfs refactor was dropped from this series if thats what you are
referring to here.
So even this and the next patch dont serve any purpose in this series
and should be dropped, Unless you have some reason of keeping them here.
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 39 +++
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 1d9d83d7b99e..333dcfe57800 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -206,33 +206,38 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
return 0;
}
-static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
- struct drm_crtc *crtc)
+static void dpu_core_perf_aggregate(struct drm_device *ddev,
+ enum dpu_crtc_client_type curr_client_type,
+ struct dpu_core_perf_params *perf)
{
- struct dpu_core_perf_params perf = { 0 };
- enum dpu_crtc_client_type curr_client_type
- = dpu_crtc_get_client_type(crtc);
- struct drm_crtc *tmp_crtc;
struct dpu_crtc_state *dpu_cstate;
- int i, ret = 0;
- u64 avg_bw;
+ struct drm_crtc *tmp_crtc;
- drm_for_each_crtc(tmp_crtc, crtc->dev) {
+ drm_for_each_crtc(tmp_crtc, ddev) {
if (tmp_crtc->enabled &&
- curr_client_type ==
- dpu_crtc_get_client_type(tmp_crtc)) {
+ curr_client_type == dpu_crtc_get_client_type(tmp_crtc)) {
dpu_cstate = to_dpu_crtc_state(tmp_crtc->state);
- perf.max_per_pipe_ib = max(perf.max_per_pipe_ib,
- dpu_cstate->new_perf.max_per_pipe_ib);
+ perf->max_per_pipe_ib = max(perf->max_per_pipe_ib,
+
dpu_cstate->new_perf.max_per_pipe_ib);
- perf.bw_ctl += dpu_cstate->new_perf.bw_ctl;
+ perf->bw_ctl += dpu_cstate->new_perf.bw_ctl;
- DRM_DEBUG_ATOMIC("crtc=%d bw=%llu paths:%d\n",
- tmp_crtc->base.id,
- dpu_cstate->new_perf.bw_ctl, kms->num_paths);
+ DRM_DEBUG_ATOMIC("crtc=%d bw=%llu\n",
+tmp_crtc->base.id,
+dpu_cstate->new_perf.bw_ctl);
}
}
+}
+
+static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
+ struct drm_crtc *crtc)
+{
+ struct dpu_core_perf_params perf = { 0 };
+ int i, ret = 0;
+ u64 avg_bw;
+
+ dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc),
);
if (!kms->num_paths)
return 0;
Re: [Freedreno] [PATCH v4 04/11] drm/msm/dpu: drop separate dpu_core_perf_tune overrides
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote: The values in struct dpu_core_perf_tune are fixed per the core perf mode. Drop the 'tune' values and substitute them with known values when performing perf management. Note: min_bus_vote was not used at all, so it is just silently dropped. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 27 --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 --- 2 files changed, 11 insertions(+), 20 deletions(-) Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v4 07/11] drm/msm/dpu: use dpu_perf_cfg in DPU core_perf code
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote: Simplify dpu_core_perf code by using only dpu_perf_cfg instead of using full-featured catalog data. Acked-by: Konrad Dybcio Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 73 --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +- 3 files changed, 35 insertions(+), 48 deletions(-) With dpu_core_rev as a separate struct, if we do need the dpu hw version in this module, we can just pass that instead. Hence, Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v4 10/11] drm/msm/dpu: move max clock decision to dpu_kms.
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote: dpu_core_perf should not make decisions on the maximum possible core clock rate. Pass the value from dpu_kms_hw_init(). Signed-off-by: Dmitry Baryshkov Also mention that with this, you are dropping the now unused core_clk handle in the dpu_core_perf. With that, Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v4 11/11] drm/msm/dpu: drop dpu_core_perf_destroy()
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote: This function does nothing, just clears one struct field. Drop it now. Acked-by: Konrad Dybcio Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 10 -- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 6 -- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 - 3 files changed, 17 deletions(-) Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v4 09/11] drm/msm/dpu: remove extra clk_round_rate() call
On 7/12/2023 3:11 PM, Dmitry Baryshkov wrote: The dev_pm_opp_set_rate() already contains a call for clk_round_rate for the passed value. Stop calling it manually from _dpu_core_perf_get_core_clk_rate(). It is slightly incorrect to call it this way, as we should round the final calculated clock rate rather than rounding all the intermediate values. Signed-off-by: Dmitry Baryshkov --- Reviewed-by: Abhinav Kumar
Re: [Freedreno] [PATCH v1 4/5] drm/msm/dp: move relevant dp initialization code from bind() to probe()
On 17/07/2023 20:16, Kuogee Hsieh wrote:
On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
On 10/07/2023 19:57, Kuogee Hsieh wrote:
On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
On 08/07/2023 02:52, Kuogee Hsieh wrote:
In preparation of moving edp of_dp_aux_populate_bus() to
dp_display_probe(), move dp_display_request_irq(),
dp->parser->parse() and dp_power_client_init() to dp_display_probe()
too.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 48
+
drivers/gpu/drm/msm/dp/dp_display.h | 1 -
2 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 44580c2..185f1eb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev,
struct device *master,
goto end;
}
- rc = dp_power_client_init(dp->power);
- if (rc) {
- DRM_ERROR("Power client create failed\n");
- goto end;
- }
-
rc = dp_register_audio_driver(dev, dp->audio);
if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
@@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp->parser->parse(dp->parser);
+ if (rc) {
+ DRM_ERROR("device tree parsing failed\n");
+ goto error;
+ }
+
dp->catalog = dp_catalog_get(dev, >parser->io);
if (IS_ERR(dp->catalog)) {
rc = PTR_ERR(dp->catalog);
@@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp_power_client_init(dp->power);
+ if (rc) {
+ DRM_ERROR("Power client create failed\n");
+ goto error;
+ }
+
dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
if (IS_ERR(dp->aux)) {
rc = PTR_ERR(dp->aux);
@@ -1196,26 +1202,20 @@ static irqreturn_t
dp_display_irq_handler(int irq, void *dev_id)
return ret;
}
-int dp_display_request_irq(struct msm_dp *dp_display)
+static int dp_display_request_irq(struct dp_display_private *dp)
{
int rc = 0;
- struct dp_display_private *dp;
-
- if (!dp_display) {
- DRM_ERROR("invalid input\n");
- return -EINVAL;
- }
-
- dp = container_of(dp_display, struct dp_display_private,
dp_display);
+ struct device *dev = >pdev->dev;
- dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
if (!dp->irq) {
- DRM_ERROR("failed to get irq\n");
- return -EINVAL;
+ dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
+ if (!dp->irq) {
+ DRM_ERROR("failed to get irq\n");
+ return -EINVAL;
+ }
}
Use platform_get_irq() from probe() function.
- rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
- dp_display_irq_handler,
+ rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
if (rc < 0) {
DRM_ERROR("failed to request IRQ%u: %d\n",
@@ -1290,6 +1290,8 @@ static int dp_display_probe(struct
platform_device *pdev)
platform_set_drvdata(pdev, >dp_display);
+ dp_display_request_irq(dp);
+
Error checking?
Are we completely ready to handle interrupts at this point?
not until dp_display_host_init() is called which will be called from
pm_runtime_resume() later.
But once you request_irq(), you should be ready for the IRQs to be
delivered right away.
At this point, the DP controller interrupts mask bit is not enabled yet.
Therefore interrupts will not happen until dp_bridge_hpd_enable() is
called to initialize dp host controller and then enabled mask bits.
Are AUX and CTRL interrupts also disabled? What about any stray/pending
interrupts? Just take it as a rule of thumb. Once request_irq() has been
called without the IRQ_NOAUTOEN flag, the driver should be prepared to
handle the incoming interrupt requests.
rc = component_add(>dev, _display_comp_ops);
if (rc) {
DRM_ERROR("component add failed, rc=%d\n", rc);
@@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp
*dp_display, struct drm_device *dev,
dp_priv = container_of(dp_display, struct
dp_display_private, dp_display);
- ret = dp_display_request_irq(dp_display);
- if (ret) {
- DRM_ERROR("request_irq failed, ret=%d\n", ret);
- return ret;
- }
-
ret = dp_display_get_next_bridge(dp_display);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 1e9415a..b3c08de 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -35,7 +35,6 @@ struct msm_dp {
int dp_display_set_plugged_cb(struct msm_dp *dp_display,
hdmi_codec_plugged_cb fn, struct device *codec_dev);
Re: [Freedreno] [PATCH v1 4/5] drm/msm/dp: move relevant dp initialization code from bind() to probe()
On 7/10/2023 11:13 AM, Dmitry Baryshkov wrote:
On 10/07/2023 19:57, Kuogee Hsieh wrote:
On 7/7/2023 5:11 PM, Dmitry Baryshkov wrote:
On 08/07/2023 02:52, Kuogee Hsieh wrote:
In preparation of moving edp of_dp_aux_populate_bus() to
dp_display_probe(), move dp_display_request_irq(),
dp->parser->parse() and dp_power_client_init() to dp_display_probe()
too.
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 48
+
drivers/gpu/drm/msm/dp/dp_display.h | 1 -
2 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 44580c2..185f1eb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -290,12 +290,6 @@ static int dp_display_bind(struct device *dev,
struct device *master,
goto end;
}
- rc = dp_power_client_init(dp->power);
- if (rc) {
- DRM_ERROR("Power client create failed\n");
- goto end;
- }
-
rc = dp_register_audio_driver(dev, dp->audio);
if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
@@ -752,6 +746,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp->parser->parse(dp->parser);
+ if (rc) {
+ DRM_ERROR("device tree parsing failed\n");
+ goto error;
+ }
+
dp->catalog = dp_catalog_get(dev, >parser->io);
if (IS_ERR(dp->catalog)) {
rc = PTR_ERR(dp->catalog);
@@ -768,6 +768,12 @@ static int dp_init_sub_modules(struct
dp_display_private *dp)
goto error;
}
+ rc = dp_power_client_init(dp->power);
+ if (rc) {
+ DRM_ERROR("Power client create failed\n");
+ goto error;
+ }
+
dp->aux = dp_aux_get(dev, dp->catalog, dp->dp_display.is_edp);
if (IS_ERR(dp->aux)) {
rc = PTR_ERR(dp->aux);
@@ -1196,26 +1202,20 @@ static irqreturn_t
dp_display_irq_handler(int irq, void *dev_id)
return ret;
}
-int dp_display_request_irq(struct msm_dp *dp_display)
+static int dp_display_request_irq(struct dp_display_private *dp)
{
int rc = 0;
- struct dp_display_private *dp;
-
- if (!dp_display) {
- DRM_ERROR("invalid input\n");
- return -EINVAL;
- }
-
- dp = container_of(dp_display, struct dp_display_private,
dp_display);
+ struct device *dev = >pdev->dev;
- dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
if (!dp->irq) {
- DRM_ERROR("failed to get irq\n");
- return -EINVAL;
+ dp->irq = irq_of_parse_and_map(dp->pdev->dev.of_node, 0);
+ if (!dp->irq) {
+ DRM_ERROR("failed to get irq\n");
+ return -EINVAL;
+ }
}
Use platform_get_irq() from probe() function.
- rc = devm_request_irq(dp_display->drm_dev->dev, dp->irq,
- dp_display_irq_handler,
+ rc = devm_request_irq(dev, dp->irq, dp_display_irq_handler,
IRQF_TRIGGER_HIGH, "dp_display_isr", dp);
if (rc < 0) {
DRM_ERROR("failed to request IRQ%u: %d\n",
@@ -1290,6 +1290,8 @@ static int dp_display_probe(struct
platform_device *pdev)
platform_set_drvdata(pdev, >dp_display);
+ dp_display_request_irq(dp);
+
Error checking?
Are we completely ready to handle interrupts at this point?
not until dp_display_host_init() is called which will be called from
pm_runtime_resume() later.
But once you request_irq(), you should be ready for the IRQs to be
delivered right away.
At this point, the DP controller interrupts mask bit is not enabled yet.
Therefore interrupts will not happen until dp_bridge_hpd_enable() is
called to initialize dp host controller and then enabled mask bits.
rc = component_add(>dev, _display_comp_ops);
if (rc) {
DRM_ERROR("component add failed, rc=%d\n", rc);
@@ -1574,12 +1576,6 @@ int msm_dp_modeset_init(struct msm_dp
*dp_display, struct drm_device *dev,
dp_priv = container_of(dp_display, struct
dp_display_private, dp_display);
- ret = dp_display_request_irq(dp_display);
- if (ret) {
- DRM_ERROR("request_irq failed, ret=%d\n", ret);
- return ret;
- }
-
ret = dp_display_get_next_bridge(dp_display);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 1e9415a..b3c08de 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -35,7 +35,6 @@ struct msm_dp {
int dp_display_set_plugged_cb(struct msm_dp *dp_display,
hdmi_codec_plugged_cb fn, struct device *codec_dev);
int dp_display_get_modes(struct msm_dp *dp_display);
-int dp_display_request_irq(struct msm_dp *dp_display);
bool dp_display_check_video_test(struct msm_dp *dp_display);
int dp_display_get_test_bpp(struct msm_dp *dp_display);
void dp_display_signal_audio_start(struct msm_dp *dp_display);
Re: [Freedreno] [PATCH] drm: Explicitly include correct DT includes
On Mon, Jul 17, 2023 at 4:27 PM Rob Herring wrote: > > On Sun, Jul 16, 2023 at 3:26 AM Heiko Stuebner wrote: > > > > Am Freitag, 14. Juli 2023, 19:45:34 CEST schrieb Rob Herring: > > > The DT of_device.h and of_platform.h date back to the separate > > > of_platform_bus_type before it as merged into the regular platform bus. > > > As part of that merge prepping Arm DT support 13 years ago, they > > > "temporarily" include each other. They also include platform_device.h > > > and of.h. As a result, there's a pretty much random mix of those include > > > files used throughout the tree. In order to detangle these headers and > > > replace the implicit includes with struct declarations, users need to > > > explicitly include the correct includes. > > > > > > Signed-off-by: Rob Herring > > > --- > > > > [...] > > > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > > index 917e79951aac..2744d8f4a6fa 100644 > > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > > @@ -12,7 +12,9 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > +#include > > > #include > > > #include > > > > I'm not sure if I'm just misreading something, but in all other places > > of_device.h gets removed while here is stays as an include. Is this > > correct this way? > > Yes, because of_match_device() is used. > > Rob > For drivers/gpu/drm/bridge/ Acked-by: Robert Foss
Re: [Freedreno] [PATCH] drm: Explicitly include correct DT includes
On Sun, Jul 16, 2023 at 3:26 AM Heiko Stuebner wrote: > > Am Freitag, 14. Juli 2023, 19:45:34 CEST schrieb Rob Herring: > > The DT of_device.h and of_platform.h date back to the separate > > of_platform_bus_type before it as merged into the regular platform bus. > > As part of that merge prepping Arm DT support 13 years ago, they > > "temporarily" include each other. They also include platform_device.h > > and of.h. As a result, there's a pretty much random mix of those include > > files used throughout the tree. In order to detangle these headers and > > replace the implicit includes with struct declarations, users need to > > explicitly include the correct includes. > > > > Signed-off-by: Rob Herring > > --- > > [...] > > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > index 917e79951aac..2744d8f4a6fa 100644 > > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c > > @@ -12,7 +12,9 @@ > > #include > > #include > > #include > > +#include > > #include > > +#include > > #include > > #include > > I'm not sure if I'm just misreading something, but in all other places > of_device.h gets removed while here is stays as an include. Is this > correct this way? Yes, because of_match_device() is used. Rob
Re: [Freedreno] [PATCH] drm: Explicitly include correct DT includes
Quoting Rob Herring (2023-07-14 18:45:34) > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring > drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 2 +- > drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 2 ++ > drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 1 + > drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 1 - > drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c | 1 - For drivers/gpu/drm/renesas/rcar-du/ > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > index 1ffde19cb87f..3904b0cca814 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > @@ -12,7 +12,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > index adfb36b0e815..9ff4537c26c8 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > @@ -20,8 +20,10 @@ > > #include > #include > +#include > #include > #include > +#include > #include > > #include "rcar_du_crtc.h" > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > index 45c05d0ffc70..9cbb5e6e2cba 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > index e10e4d4b89a2..db2e6f16f954 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > @@ -12,7 +12,6 @@ > #include > #include > #include > -#include > #include > #include > #include > diff --git a/drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c > b/drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c > index aa95b85a2964..8048bdca2d6c 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c > @@ -10,7 +10,6 @@ > #include > #include > #include > -#include > #include > #include > #include Reviewed-by: Kieran Bingham
Re: [Freedreno] [PATCH] drm: Explicitly include correct DT includes
On Fri, Jul 14, 2023 at 11:45:34AM -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring > --- > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > index cc7664c95a54..14ee79becacb 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c > @@ -6,7 +6,7 @@ > */ > #include > #include > -#include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index c03cfd57b752..a5a9534d4353 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > For the komeda and malidp drivers: Acked-by: Liviu Dudau Best regards, Liviu > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c > index 99964f5a5457..2a6b91f752cb 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c > @@ -7,7 +7,6 @@ > > #include > #include > -#include > #include > #include > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 2254457ab5d0..b9957da0f55a 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -9,7 +9,7 @@ > #include > #include > #include > -#include > +#include > #include > > #include > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > index f50d65f54314..7457d38622b0 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > @@ -14,8 +14,7 @@ > #include > #include > #include > -#include > -#include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > index f6822dfa3805..4aff817f82ce 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > @@ -29,7 +29,6 @@ > #include > #include > #include > -#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c > b/drivers/gpu/drm/bridge/chipone-icn6211.c > index 8bfce21d6b90..d205e755e524 100644 > --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > @@ -17,7 +17,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > > diff --git a/drivers/gpu/drm/bridge/display-connector.c > b/drivers/gpu/drm/bridge/display-connector.c > index f7f436cf96e0..08bd5695ddae 100644 > --- a/drivers/gpu/drm/bridge/display-connector.c > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -10,7 +10,6 @@ > #include > #include > #include > -#include > #include > #include > > diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c > b/drivers/gpu/drm/bridge/fsl-ldb.c > index b8e52156b07a..0e4bac7dd04f 100644 > --- a/drivers/gpu/drm/bridge/fsl-ldb.c > +++ b/drivers/gpu/drm/bridge/fsl-ldb.c > @@ -8,7 +8,6 @@ > #include > #include > #include > -#include > #include > #include > #include > diff --git a/drivers/gpu/drm/bridge/imx/imx8qm-ldb.c > b/drivers/gpu/drm/bridge/imx/imx8qm-ldb.c > index 386032a02599..21471a9a28b2 100644 > --- a/drivers/gpu/drm/bridge/imx/imx8qm-ldb.c > +++ b/drivers/gpu/drm/bridge/imx/imx8qm-ldb.c > @@ -9,9 +9,9 @@ > #include > #include > #include > -#include > #include > #include > +#include > #include > #include > > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c > b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c > index c806576b1e22..7984da9c0a35 100644 > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > > diff --git a/drivers/gpu/drm/bridge/lontium-lt9211.c > b/drivers/gpu/drm/bridge/lontium-lt9211.c > index aa8d47e7f40d..4d404f5ef87e 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9211.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9211.c > @@ -16,7 +16,6 @@ > #include > #include > #include >
