Re: [Freedreno] [PATCH] Revert "drm/msm/dp: Remove INIT_SETUP delay"

2023-06-01 Thread Leonard Lausen
Hi Abhinav,

June 1, 2023 at 3:20 PM, "Abhinav Kumar"  wrote:
> > 
> >  [drm:drm_mode_config_helper_resume] *ERROR* Failed to resume (-107)
> > 
> 
> We are not able to recreate this on sc7280 chromebooks , will need to check 
> on sc7180. This does not seem directly related to any of the hotplug changes 
> though so needs to be checked separately. So please feel free to raise a 
> gitlab bug for this and assign to me.

Thank you for checking with sc7280. I created 
https://gitlab.freedesktop.org/drm/msm/-/issues/25 and CCed you. I've also 
verified that the error persists with v6.4.0-rc4 + Kuogee's patch (just in case 
you may have tested on sc7280 with 6.4).
 
> >  https://patchwork.freedesktop.org/patch/538601/?series=118148=3
> >  Apologies if you were not CCed on this, if a next version is CCed,
> >  will ask kuogee to cc you.
> >  Meanwhile, will be great if you can verify if it works for you and
> >  provide Tested-by tags.

I see Bjorn also tested the patch. As it fixes a serious USB-C DP regression 
which broke USB-C DP completely on lazor for v6.3, can it be included in 
upcoming 6.3.y release?

Thank you
Leonard


Re: [Freedreno] [PATCH v2 1/2] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

2023-06-01 Thread Dmitry Baryshkov

On 02/06/2023 01:08, Kuogee Hsieh wrote:

At current implementation, DSI DSC struct is populated at display setup
during system bootup. This mechanism works fine with embedded display.
But will run into problem with plugin/unplug oriented external display,
such as DP, due to DSC struct will become stale once external display
unplugged. New DSC struct has to be re populated to reflect newer external
display which just plugged in. Move retrieving of DSI DSC struct to
atomic_check() so that same mechanism will work for both embedded display
and external plugin/unplug oriented display.

Signed-off-by: Kuogee Hsieh 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3b416e1..5c440a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -604,7 +604,7 @@ static int dpu_encoder_virt_atomic_check(
struct drm_display_mode *adj_mode;
struct msm_display_topology topology;
struct dpu_global_state *global_state;
-   int i = 0;
+   int index, i = 0;
int ret = 0;
  
  	if (!drm_enc || !crtc_state || !conn_state) {

@@ -639,6 +639,10 @@ static int dpu_encoder_virt_atomic_check(
}
}
  
+	index = dpu_enc->disp_info.h_tile_instance[0];

+if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+   dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);


As discussed previously, one should not write to non-state objects from 
atomic_check. This chunk does.


Not to mention that this will start exploding once you try adding DP 
next to it.


Please abstain from posting next revisions until the discussions on the 
previous one are more or less finished. For now this is NAK.


Not to mention that this patch doesn't pass checkpatch.pl.


+
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, 
crtc_state);
  
  	/*

@@ -1034,7 +1038,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct 
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
int num_lm, num_ctl, num_pp, num_dsc;
unsigned int dsc_mask = 0;
-   int i;
+   int index, i;
  
  	if (!drm_enc) {

DPU_ERROR("invalid encoder\n");
@@ -1055,6 +1059,10 @@ static void dpu_encoder_virt_atomic_mode_set(struct 
drm_encoder *drm_enc,
  
  	trace_dpu_enc_mode_set(DRMID(drm_enc));
  
+	index = dpu_enc->disp_info.h_tile_instance[0];

+if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+   dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);


Doesn't this seem 100% same as the previous chunk? Doesn't it plead to 
be extracted to a helper function?



+
/* Query resource that have been reserved in atomic check step. */
num_pp = dpu_rm_get_assigned_resources(_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_PINGPONG, hw_pp,
@@ -2121,8 +2129,10 @@ void dpu_encoder_helper_phys_cleanup(struct 
dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}
  
-	if (dpu_enc->dsc)

+   if (dpu_enc->dsc) {
dpu_encoder_unprep_dsc(dpu_enc);
+   dpu_enc->dsc = NULL;
+   }
  
  	intf_cfg.stream_sel = 0; /* Don't care value for video mode */

intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);


--
With best wishes
Dmitry



Re: [Freedreno] [PATCH v2 0/2] retrieve DSI DSC through DRM bridge

2023-06-01 Thread Dmitry Baryshkov

On 02/06/2023 01:08, Kuogee Hsieh wrote:

move retrieving DSC from setup_display to atomic_check() and delete struct 
drm_dsc_config
from struct msm_display_info.


This is obvious from the patches themselves. You should be describing 
_why_ the changes are necessary, not what is changed.



What was changed between v1 and v2?



Kuogee Hsieh (2):
   drm/msm/dpu: retrieve DSI DSC struct at atomic_check()
   drm/msm/dpu: remove struct drm_dsc_config from struct msm_display_info

  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  1 -
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  2 --
  3 files changed, 13 insertions(+), 8 deletions(-)



--
With best wishes
Dmitry



[Freedreno] [PATCH v2 2/2] drm/msm/dpu: remove struct drm_dsc_config from struct msm_display_info

2023-06-01 Thread Kuogee Hsieh
Since struct drm_dsc_config is retrieved at atomic_check() instead of at
display setup time during bootup. Saving struct drm_dsc_config at
struct msm_display_info is not necessary and become redundant.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 --
 3 files changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5c440a0..53274a5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2332,8 +2332,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
dpu_enc->idle_pc_supported =
dpu_kms->catalog->caps->has_idle_pc;
 
-   dpu_enc->dsc = disp_info->dsc;
-
mutex_lock(_enc->enc_lock);
for (i = 0; i < disp_info->num_of_h_tiles && !ret; i++) {
/*
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 2c9ef8d..50e64cf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -36,7 +36,6 @@ struct msm_display_info {
uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
bool is_cmd_mode;
bool is_te_using_watchdog_timer;
-   struct drm_dsc_config *dsc;
 };
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index c24f487..2390e5c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -554,8 +554,6 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
info.h_tile_instance[info.num_of_h_tiles++] = i;
info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
 
-   info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]);
-
if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
rc = msm_dsi_modeset_init(priv->dsi[other], dev, 
encoder);
if (rc) {
-- 
2.7.4



[Freedreno] [PATCH v2 1/2] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

2023-06-01 Thread Kuogee Hsieh
At current implementation, DSI DSC struct is populated at display setup
during system bootup. This mechanism works fine with embedded display.
But will run into problem with plugin/unplug oriented external display,
such as DP, due to DSC struct will become stale once external display
unplugged. New DSC struct has to be re populated to reflect newer external
display which just plugged in. Move retrieving of DSI DSC struct to
atomic_check() so that same mechanism will work for both embedded display
and external plugin/unplug oriented display.

Signed-off-by: Kuogee Hsieh 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3b416e1..5c440a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -604,7 +604,7 @@ static int dpu_encoder_virt_atomic_check(
struct drm_display_mode *adj_mode;
struct msm_display_topology topology;
struct dpu_global_state *global_state;
-   int i = 0;
+   int index, i = 0;
int ret = 0;
 
if (!drm_enc || !crtc_state || !conn_state) {
@@ -639,6 +639,10 @@ static int dpu_encoder_virt_atomic_check(
}
}
 
+   index = dpu_enc->disp_info.h_tile_instance[0];
+if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+   dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);
+
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, 
crtc_state);
 
/*
@@ -1034,7 +1038,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct 
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
int num_lm, num_ctl, num_pp, num_dsc;
unsigned int dsc_mask = 0;
-   int i;
+   int index, i;
 
if (!drm_enc) {
DPU_ERROR("invalid encoder\n");
@@ -1055,6 +1059,10 @@ static void dpu_encoder_virt_atomic_mode_set(struct 
drm_encoder *drm_enc,
 
trace_dpu_enc_mode_set(DRMID(drm_enc));
 
+   index = dpu_enc->disp_info.h_tile_instance[0];
+if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+   dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);
+
/* Query resource that have been reserved in atomic check step. */
num_pp = dpu_rm_get_assigned_resources(_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_PINGPONG, hw_pp,
@@ -2121,8 +2129,10 @@ void dpu_encoder_helper_phys_cleanup(struct 
dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}
 
-   if (dpu_enc->dsc)
+   if (dpu_enc->dsc) {
dpu_encoder_unprep_dsc(dpu_enc);
+   dpu_enc->dsc = NULL;
+   }
 
intf_cfg.stream_sel = 0; /* Don't care value for video mode */
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
-- 
2.7.4



[Freedreno] [PATCH v2 0/2] retrieve DSI DSC through DRM bridge

2023-06-01 Thread Kuogee Hsieh
move retrieving DSC from setup_display to atomic_check() and delete struct 
drm_dsc_config
from struct msm_display_info.

Kuogee Hsieh (2):
  drm/msm/dpu: retrieve DSI DSC struct at atomic_check()
  drm/msm/dpu: remove struct drm_dsc_config from struct msm_display_info

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  2 --
 3 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.7.4



Re: [Freedreno] [PATCH v3 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

There is no need to assign a result to temp varable just to return it
after a goto. Drop the temporary variable and goto and return the result
directly.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

There is no need to assign a result to temp varable just to return it
two lines below. Drop the temporary variable.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

There is little sense to get intf index just to call dpu_rm_get_intf()
on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 4/7] drm/msm/dpu: inline dpu_encoder_get_wb()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

The function dpu_encoder_get_wb() returns controller_id if the
corresponding WB is present in the catalog. We can inline this function
and rely on dpu_rm_get_wb() returning NULL for indices for which the
WB is not present on the device.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and
struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and
use them to get the instance index.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 2/7] drm/msm/dpu: separate common function to init physical encoder

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---


Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH v3 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

There is no reason to split the dpu_encoder interface into separate
_init() and _setup() phases. Merge them into a single function.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---



Tested-by: Abhinav Kumar  # sc7280


Re: [Freedreno] [PATCH] Revert "drm/msm/dp: Remove INIT_SETUP delay"

2023-06-01 Thread Abhinav Kumar

Hi Leonard

On 5/24/2023 5:58 AM, Leonard Lausen wrote:

[  275.025497] [drm:dpu_encoder_phys_vid_wait_for_commit_done:488]
[dpu error]vblank timeout
[  275.025514] [drm:dpu_kms_wait_for_commit_done:510] [dpu error]wait
for commit done returned -110
[  275.064141] [drm:dpu_encoder_frame_done_timeout:2382] [dpu
error]enc33 frame done timeout


This is a different crash but the root-cause of both the issues is the
bridge hpd_enable/disable series.

https://patchwork.freedesktop.org/patch/514414/


Yes, the new patch to fix this issue is here

https://patchwork.freedesktop.org/patch/538601/?series=118148=3

Apologies if you were not CCed on this, if a next version is CCed,
will ask kuogee to cc you.

Meanwhile, will be great if you can verify if it works for you and
provide Tested-by tags.


Hi Leonard,

I had  cc you with v5 patches.

Would you please verify it.


Hi Kuogee,

thank you. Verified the v6 patch fixes the regression when ported to
6.3.3. One non-fatal issue remains: Suspending and resuming the system
while USB-C DP monitor is connected triggers an error, though the system
recovers within a second without the need to unplug the cable.

[drm:drm_mode_config_helper_resume] *ERROR* Failed to resume (-107)



We are not able to recreate this on sc7280 chromebooks , will need to 
check on sc7180. This does not seem directly related to any of the 
hotplug changes though so needs to be checked separately. So please feel 
free to raise a gitlab bug for this and assign to me.


Re: [Freedreno] [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

2023-06-01 Thread Dmitry Baryshkov
On Thu, 1 Jun 2023 at 20:37, Kuogee Hsieh  wrote:
>
>
> On 5/31/2023 10:53 AM, Dmitry Baryshkov wrote:
> > On Wed, 31 May 2023 at 20:29, Kuogee Hsieh  wrote:
> >>
> >> On 5/31/2023 10:12 AM, Dmitry Baryshkov wrote:
> >>> On Wed, 31 May 2023 at 18:41, Kuogee Hsieh  
> >>> wrote:
> 
> >> +if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
> > INTF_DSI
> >
> >> +struct drm_bridge *bridge;
> >> +
> >> +if (!dpu_enc->dsc) {
> > This condition is not correct. We should be updating the DSC even if
> > there is one.
> >
> >> +bridge = drm_bridge_chain_get_first_bridge(drm_enc);
> >> +dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
> > This approach will not work for the hot-pluggable outputs. The dpu_enc
> > is not a part of the state. It should not be touched before
> > atomic_commit actually commits changes.
>  where can drm_dsc_config be stored?
> >>> I'd say, get it during atomic_check (and don't store it anywhere).
> >>> Then get it during atomic_enable (and save in dpu_enc).
> >> got it.
> > Also, I don't think I like the API. It makes it impossible for the
> > driver to check that the bridge is the actually our DSI bridge or not.
> > Once you add DP here, the code will explode.
> >
> > I think instead we should extend the drm_bridge API to be able to get
> > the DSC configuration from it directly. Additional care should be put
> > to design an assymetrical API. Theoretically a drm_bridge can be both
> > DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge,
> > supporting DSC on the DSI side too.
>  Form my understanding, a bridge contains two interfaces.
> 
>  Therefore I would think only one bridge for dsi-to-dp bridge? and this
>  bridge should represent the bridge chip?
> 
>  I am thinking adding an ops function, get_bridge_dsc() to struct
>  drm_bridge_funcs to retrieve drm_dsc_config.
> >>> So, for this DSI-to-DP bridge will get_bridge_dsc() return DSC
> >>> configuration for  the DSI or for the DP side of the bridge?
> >> I would think should be DP side. there is no reason to enable dsc on
> >> both DSI and DP fro a bridge chip.
>
> My above statement is not correct. For DSI-to-DP bridge,
> drm_bridge_chain_get_first_bridge(drm_enc) return DSI bridge.
>
> Is possible that DSC feature can be added to DSI-to-DP bridge?
>
> If it is not possible, then we can rule out DSI-to-DP bridge case, then
> use struct drm_bridge to retrieve DSC form controller will work.
>
>
>
> > Well, there can be. E.g. to lower the clock rates of the DSI link.
> >
> >> drm_bridge_chain_get_first_bridge(drm_enc) should return the bridge of
> >> the controller.
> >>
> >> In DSI-to-DP bridge chip case, this controller will be the bridge chip
> >> who configured to perform protocol conversion between DSI and DP.
> >>
> >> If DSC enabled should be at DP size which connect to panel.
> > Ok, so it returns the DSC configuration of the bridge's source side.
> > Now let's consider a panel bridge for the DSC-enabled DSI panel.
> > Should get_bridge_dsc() return a DSC config in this case?
> >
>  Do you have other suggestion?
> >>> Let me think about it for a few days.
>
> There is one option which is keep current
>
> 1) keep struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi
> *msm_dsi) at dsi.c
>
> 2) use  struct msm_display_info *disp_info saved at dpu_enc to locate
> struct msm_dsi from priv->dsi[] list (see item #3)
>
> 3)  info.dsc = msm_dsi_get_dsc_config(priv->dsi[info.h_tile_instance[0]]);
>
> 4) ballistically, keep original code but move  info.dsc =
> msm_dsi_get_dsc_config(priv->dsi[i]); to other place sush as
> atomic_check() and atomic_enable().
>

5) leave drm_dsc_config handling as is, update the dsc config from the
DP driver as suitable. If DSC is not supported, set
dsc->dsc_version_major = 0 and dsc->dsc_version_minor = 0 on the DP
side. In DPU driver verify that dsc->dsc_version_major/_minor != 0.

-- 
With best wishes
Dmitry


Re: [Freedreno] [PATCH v2 2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes

2023-06-01 Thread Akhil P Oommen
On Tue, May 30, 2023 at 08:35:14AM -0700, Bjorn Andersson wrote:
> 
> On Mon, May 29, 2023 at 02:16:14PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, May 29, 2023 at 09:38:59AM +0200, Konrad Dybcio wrote:
> > > On 28.05.2023 19:07, Manivannan Sadhasivam wrote:
> > > > On Tue, May 23, 2023 at 09:59:53AM +0200, Konrad Dybcio wrote:
> > > >> On 23.05.2023 03:15, Bjorn Andersson wrote:
> > > >>> From: Bjorn Andersson 
> [..]
> > > >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi 
> > > >>> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> [..]
> > > >>> + gmu: gmu@3d6a000 {
> [..]
> > > >>> + status = "disabled";
> > > >> I've recently discovered that - and I am not 100% sure - all GMUs are
> > > >> cache-coherent. Could you please ask somebody at qc about this?
> > > >>
> > > > 
> > > > AFAIU, GMU's job is controlling the voltage and clock to the GPU.
> > > Not just that, it's only the limited functionality we've implemented
> > > upstream so far.
> > > 
> > 
> > Okay, good to know!
> > 
> > > It doesn't do
> > > > any data transactions on its own.
> > > Of course it does. AP communication is done through MMIO writes and
> > > the GMU talks to RPMh via the GPU RSC directly. Apart from that, some
> > > of the GPU registers (that nota bene don't have anything to do with
> > > the GMU M3 core itself) lay within the GMU address space.
> > > 
> 
> But those aren't shared memory accesses.
> 
> > 
> > That doesn't justify the fact that cache coherency is needed, especially
> > MMIO writes, unless GMU could snoop the MMIO writes to AP caches.
> > 
> 
> In reviewing the downstream state again I noticed that the GPU smmu is
> marked dma-coherent, so I will adjust that in v3.
Bjorn,

Would you mind sharing a perf delta (preferrably manhattan offscreen)
you see with and without this dma-coherent property?

-Akhil.
> 
> Regards,
> Bjorn


Re: [Freedreno] [PATCH v2 2/3] arm64: dts: qcom: sc8280xp: Add GPU related nodes

2023-06-01 Thread Akhil P Oommen
On Mon, May 29, 2023 at 09:38:59AM +0200, Konrad Dybcio wrote:
> 
> 
> 
> On 28.05.2023 19:07, Manivannan Sadhasivam wrote:
> > On Tue, May 23, 2023 at 09:59:53AM +0200, Konrad Dybcio wrote:
> >>
> >>
> >> On 23.05.2023 03:15, Bjorn Andersson wrote:
> >>> From: Bjorn Andersson 
> >>>
> >>> Add Adreno SMMU, GPU clock controller, GMU and GPU nodes for the
> >>> SC8280XP.
> >>>
> >>> Signed-off-by: Bjorn Andersson 
> >>> Signed-off-by: Bjorn Andersson 
> >>> ---
> >> It does not look like you tested the DTS against bindings. Please run
> >> `make dtbs_check` (see
> >> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> >>
> >>>
> >>> Changes since v1:
> >>> - Dropped gmu_pdc_seq region from , as it shouldn't have been used.
> >>> - Added missing compatible to _smmu.
> >>> - Dropped aoss_qmp clock in  and _smmu.
> >>>  
> >>>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 169 +
> >>>  1 file changed, 169 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi 
> >>> b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >>> index d2a2224d138a..329ec2119ecf 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >>> @@ -6,6 +6,7 @@
> >>>  
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> @@ -2331,6 +2332,174 @@ tcsr: syscon@1fc {
> >>>   reg = <0x0 0x01fc 0x0 0x3>;
> >>>   };
> >>>  
> >>> + gpu: gpu@3d0 {
> >>> + compatible = "qcom,adreno-690.0", "qcom,adreno";
> >>> +
> >>> + reg = <0 0x03d0 0 0x4>,
> >>> +   <0 0x03d9e000 0 0x1000>,
> >>> +   <0 0x03d61000 0 0x800>;
> >>> + reg-names = "kgsl_3d0_reg_memory",
> >>> + "cx_mem",
> >>> + "cx_dbgc";
> >>> + interrupts = ;
> >>> + iommus = <_smmu 0 0xc00>, <_smmu 1 0xc00>;
> >>> + operating-points-v2 = <_opp_table>;
> >>> +
> >>> + qcom,gmu = <>;
> >>> + interconnects = <_noc MASTER_GFX3D 0 _virt 
> >>> SLAVE_EBI1 0>;
> >>> + interconnect-names = "gfx-mem";
> >>> + #cooling-cells = <2>;
> >>> +
> >>> + status = "disabled";
> >>> +
> >>> + gpu_opp_table: opp-table {
> >>> + compatible = "operating-points-v2";
> >>> +
> >>> + opp-27000 {
> >>> + opp-hz = /bits/ 64 <27000>;
> >>> + opp-level = 
> >>> ;
> >>> + opp-peak-kBps = <451000>;
> >>> + };
> >>> +
> >>> + opp-41000 {
> >>> + opp-hz = /bits/ 64 <41000>;
> >>> + opp-level = ;
> >>> + opp-peak-kBps = <1555000>;
> >>> + };
> >>> +
> >>> + opp-5 {
> >>> + opp-hz = /bits/ 64 <5>;
> >>> + opp-level = 
> >>> ;
> >>> + opp-peak-kBps = <1555000>;
> >>> + };
> >>> +
> >>> + opp-54700 {
> >>> + opp-hz = /bits/ 64 <54700>;
> >>> + opp-level = 
> >>> ;
> >>> + opp-peak-kBps = <1555000>;
> >>> + };
> >>> +
> >>> + opp-60600 {
> >>> + opp-hz = /bits/ 64 <60600>;
> >>> + opp-level = ;
> >>> + opp-peak-kBps = <2736000>;
> >>> + };
> >>> +
> >>> + opp-64000 {
> >>> + opp-hz = /bits/ 64 <64000>;
> >>> + opp-level = 
> >>> ;
> >>> + opp-peak-kBps = <2736000>;
> >>> + };
> >>> +
> >>> + opp-69000 {
> >>> + opp-hz = /bits/ 64 <69000>;
> >>> + opp-level = 
> >>> ;
> >>> + opp-peak-kBps = <2736000>;
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> + gmu: gmu@3d6a000 {
> >>> + compatible = "qcom,adreno-gmu-690.0", "qcom,adreno-gmu";
> >>> + reg = <0 0x03d6a000 0 0x34000>,
> >>> +   <0 0x03de 0 0x1>,
> >>> +   <0 0x0b29 0 0x1>;
> >>> + reg-names = "gmu", "rscc", "gmu_pdc";
> >>> + interrupts = ,
> >>> + 

Re: [Freedreno] [PATCH v3 1/3] drm/msm/adreno: Add Adreno A690 support

2023-06-01 Thread Akhil P Oommen
On Wed, May 31, 2023 at 10:30:09PM +0200, Konrad Dybcio wrote:
> 
> 
> 
> On 31.05.2023 05:09, Bjorn Andersson wrote:
> > From: Bjorn Andersson 
> > 
> > Introduce support for the Adreno A690, found in Qualcomm SC8280XP.
> > 
> > Tested-by: Steev Klimaszewski 
> > Reviewed-by: Konrad Dybcio 
> > Signed-off-by: Bjorn Andersson 
> > Signed-off-by: Bjorn Andersson 
> > ---
> Couple of additional nits that you may or may not incorporate:
> 
> [...]
> 
> > +   {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xF3CF},
> It would be cool if we could stop adding uppercase hex outside preprocessor
> defines..
> 
> 
> [...]
> > +   A6XX_PROTECT_RDONLY(0x0fc00, 0x01fff),
> > +   A6XX_PROTECT_NORDWR(0x11c00, 0x0), /*note: infiite range */
> typo
> 
> 
> 
> -- Questions to Rob that don't really concern this patch --
> 
> > +static void a690_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> Rob, I'll be looking into reworking these into dynamic tables.. would you
> be okay with two more additions (A730, A740) on top of this before I do that?
> The number of these funcs has risen quite a bit and we're abusing the fact
> that so far there's a 1-1 mapping of SoC-Adreno (at the current state of
> mainline, not in general)..

+1. But please leave a618 and 7c3 as it is.

-Akhil

> 
> > +{
> > +   /*
> > +* Send a single "off" entry just to get things running
> > +* TODO: bus scaling
> > +*/
> Also something I'll be looking into in the near future..
> 
> > @@ -531,6 +562,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
> > adreno_7c3_build_bw_table();
> > else if (adreno_is_a660(adreno_gpu))
> > a660_build_bw_table();
> > +   else if (adreno_is_a690(adreno_gpu))
> > +   a690_build_bw_table();
> > else
> > a6xx_build_bw_table();
> I think changing the is_adreno_... to switch statements with a gpu_model
> var would make it easier to read.. Should I also rework that?
> 
> Konrad
> 
> >  
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > index 8cff86e9d35c..e5a865024e94 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > @@ -355,6 +355,20 @@ static const struct adreno_info gpulist[] = {
> > .init = a6xx_gpu_init,
> > .zapfw = "a640_zap.mdt",
> > .hwcg = a640_hwcg,
> > +   }, {
> > +   .rev = ADRENO_REV(6, 9, 0, ANY_ID),
> > +   .revn = 690,
> > +   .name = "A690",
> > +   .fw = {
> > +   [ADRENO_FW_SQE] = "a660_sqe.fw",
> > +   [ADRENO_FW_GMU] = "a690_gmu.bin",
> > +   },
> > +   .gmem = SZ_4M,
> > +   .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > +   .init = a6xx_gpu_init,
> > +   .zapfw = "a690_zap.mdt",
> > +   .hwcg = a690_hwcg,
> > +   .address_space_size = SZ_16G,
> > },
> >  };
> >  
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h 
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index f62612a5c70f..ac9c429ca07b 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -55,7 +55,7 @@ struct adreno_reglist {
> > u32 value;
> >  };
> >  
> > -extern const struct adreno_reglist a615_hwcg[], a630_hwcg[], a640_hwcg[], 
> > a650_hwcg[], a660_hwcg[];
> > +extern const struct adreno_reglist a615_hwcg[], a630_hwcg[], a640_hwcg[], 
> > a650_hwcg[], a660_hwcg[], a690_hwcg[];
> >  
> >  struct adreno_info {
> > struct adreno_rev rev;
> > @@ -272,6 +272,11 @@ static inline int adreno_is_a660(struct adreno_gpu 
> > *gpu)
> > return gpu->revn == 660;
> >  }
> >  
> > +static inline int adreno_is_a690(struct adreno_gpu *gpu)
> > +{
> > +   return gpu->revn == 690;
> > +};
> > +
> >  /* check for a615, a616, a618, a619 or any derivatives */
> >  static inline int adreno_is_a615_family(struct adreno_gpu *gpu)
> >  {
> > @@ -280,13 +285,13 @@ static inline int adreno_is_a615_family(struct 
> > adreno_gpu *gpu)
> >  
> >  static inline int adreno_is_a660_family(struct adreno_gpu *gpu)
> >  {
> > -   return adreno_is_a660(gpu) || adreno_is_7c3(gpu);
> > +   return adreno_is_a660(gpu) || adreno_is_a690(gpu) || adreno_is_7c3(gpu);
> >  }
> >  
> >  /* check for a650, a660, or any derivatives */
> >  static inline int adreno_is_a650_family(struct adreno_gpu *gpu)
> >  {
> > -   return gpu->revn == 650 || gpu->revn == 620 || 
> > adreno_is_a660_family(gpu);
> > +   return gpu->revn == 650 || gpu->revn == 620  || 
> > adreno_is_a660_family(gpu);
> >  }
> >  
> >  u64 adreno_private_address_space_size(struct msm_gpu *gpu);


Re: [Freedreno] [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

2023-06-01 Thread Kuogee Hsieh



On 5/31/2023 10:53 AM, Dmitry Baryshkov wrote:

On Wed, 31 May 2023 at 20:29, Kuogee Hsieh  wrote:


On 5/31/2023 10:12 AM, Dmitry Baryshkov wrote:

On Wed, 31 May 2023 at 18:41, Kuogee Hsieh  wrote:



+if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {

INTF_DSI


+struct drm_bridge *bridge;
+
+if (!dpu_enc->dsc) {

This condition is not correct. We should be updating the DSC even if
there is one.


+bridge = drm_bridge_chain_get_first_bridge(drm_enc);
+dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);

This approach will not work for the hot-pluggable outputs. The dpu_enc
is not a part of the state. It should not be touched before
atomic_commit actually commits changes.

where can drm_dsc_config be stored?

I'd say, get it during atomic_check (and don't store it anywhere).
Then get it during atomic_enable (and save in dpu_enc).

got it.

Also, I don't think I like the API. It makes it impossible for the
driver to check that the bridge is the actually our DSI bridge or not.
Once you add DP here, the code will explode.

I think instead we should extend the drm_bridge API to be able to get
the DSC configuration from it directly. Additional care should be put
to design an assymetrical API. Theoretically a drm_bridge can be both
DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge,
supporting DSC on the DSI side too.

Form my understanding, a bridge contains two interfaces.

Therefore I would think only one bridge for dsi-to-dp bridge? and this
bridge should represent the bridge chip?

I am thinking adding an ops function, get_bridge_dsc() to struct
drm_bridge_funcs to retrieve drm_dsc_config.

So, for this DSI-to-DP bridge will get_bridge_dsc() return DSC
configuration for  the DSI or for the DP side of the bridge?

I would think should be DP side. there is no reason to enable dsc on
both DSI and DP fro a bridge chip.


My above statement is not correct. For DSI-to-DP bridge, 
drm_bridge_chain_get_first_bridge(drm_enc) return DSI bridge.


Is possible that DSC feature can be added to DSI-to-DP bridge?

If it is not possible, then we can rule out DSI-to-DP bridge case, then 
use struct drm_bridge to retrieve DSC form controller will work.





Well, there can be. E.g. to lower the clock rates of the DSI link.


drm_bridge_chain_get_first_bridge(drm_enc) should return the bridge of
the controller.

In DSI-to-DP bridge chip case, this controller will be the bridge chip
who configured to perform protocol conversion between DSI and DP.

If DSC enabled should be at DP size which connect to panel.

Ok, so it returns the DSC configuration of the bridge's source side.
Now let's consider a panel bridge for the DSC-enabled DSI panel.
Should get_bridge_dsc() return a DSC config in this case?


Do you have other suggestion?

Let me think about it for a few days.


There is one option which is keep current

1) keep struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi 
*msm_dsi) at dsi.c


2) use  struct msm_display_info *disp_info saved at dpu_enc to locate 
struct msm_dsi from priv->dsi[] list (see item #3)


3)  info.dsc = msm_dsi_get_dsc_config(priv->dsi[info.h_tile_instance[0]]);

4) ballistically, keep original code but move  info.dsc = 
msm_dsi_get_dsc_config(priv->dsi[i]); to other place sush as 
atomic_check() and atomic_enable().




[Freedreno] [PATCH v3 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()

2023-06-01 Thread Dmitry Baryshkov
There is no need to assign a result to temp varable just to return it
two lines below. Drop the temporary variable.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

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 2cc6b0cd2710..4f8c9187f76d 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
@@ -756,15 +756,13 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 {
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-   int ret = 0;
 
DPU_DEBUG("intf\n");
 
cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL);
if (!cmd_enc) {
-   ret = -ENOMEM;
DPU_ERROR("failed to allocate\n");
-   return ERR_PTR(ret);
+   return ERR_PTR(-ENOMEM);
}
phys_enc = _enc->base;
 
-- 
2.39.2



[Freedreno] [PATCH v3 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init()

2023-06-01 Thread Dmitry Baryshkov
There is no need to assign a result to temp varable just to return it
after a goto. Drop the temporary variable and goto and return the result
directly.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 17575591a4eb..edcac512fe68 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -693,21 +693,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
 {
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_wb *wb_enc = NULL;
-   int ret = 0;
 
DPU_DEBUG("\n");
 
if (!p || !p->parent) {
DPU_ERROR("invalid params\n");
-   ret = -EINVAL;
-   goto fail_alloc;
+   return ERR_PTR(-EINVAL);
}
 
wb_enc = kzalloc(sizeof(*wb_enc), GFP_KERNEL);
if (!wb_enc) {
DPU_ERROR("failed to allocate wb phys_enc enc\n");
-   ret = -ENOMEM;
-   goto fail_alloc;
+   return ERR_PTR(-ENOMEM);
}
 
phys_enc = _enc->base;
@@ -724,7 +721,4 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
DPU_DEBUG("Created dpu_encoder_phys for wb %d\n", phys_enc->hw_wb->idx);
 
return phys_enc;
-
-fail_alloc:
-   return ERR_PTR(ret);
 }
-- 
2.39.2



[Freedreno] [PATCH v3 2/7] drm/msm/dpu: separate common function to init physical encoder

2023-06-01 Thread Dmitry Baryshkov
Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 29 +--
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  3 ++
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 17 ++-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 17 ++-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 17 ++-
 5 files changed, 37 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d4b759e8f2ae..475b30bef72d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2321,8 +2321,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-   atomic_set(>vsync_cnt, 0);
-   atomic_set(>underrun_cnt, 0);
 
if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
phys->hw_intf = dpu_rm_get_intf(_kms->rm, 
phys->intf_idx);
@@ -2524,3 +2522,30 @@ unsigned int dpu_encoder_helper_get_dsc(struct 
dpu_encoder_phys *phys_enc)
 
return dpu_enc->dsc_mask;
 }
+
+void dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
+ struct dpu_enc_phys_init_params *p)
+{
+   int i;
+
+   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
+   phys_enc->intf_idx = p->intf_idx;
+   phys_enc->wb_idx = p->wb_idx;
+   phys_enc->parent = p->parent;
+   phys_enc->dpu_kms = p->dpu_kms;
+   phys_enc->split_role = p->split_role;
+   phys_enc->enc_spinlock = p->enc_spinlock;
+   phys_enc->enable_state = DPU_ENC_DISABLED;
+
+   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+   phys_enc->irq[i] = -EINVAL;
+
+   atomic_set(_enc->vblank_refcount, 0);
+   atomic_set(_enc->pending_kickoff_cnt, 0);
+   atomic_set(_enc->pending_ctlstart_cnt, 0);
+
+   atomic_set(_enc->vsync_cnt, 0);
+   atomic_set(_enc->underrun_cnt, 0);
+
+   init_waitqueue_head(_enc->pending_kickoff_wq);
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 90f177e43262..aa98bfb70a26 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -407,4 +407,7 @@ void dpu_encoder_frame_done_callback(
struct drm_encoder *drm_enc,
struct dpu_encoder_phys *ready_phys, u32 event);
 
+void dpu_encoder_phys_init(struct dpu_encoder_phys *phys,
+  struct dpu_enc_phys_init_params *p);
+
 #endif /* __dpu_encoder_phys_H__ */
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 d8ed85a238af..2bd806c51882 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
@@ -756,7 +756,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 {
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-   int i, ret = 0;
+   int ret = 0;
 
DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
 
@@ -767,28 +767,17 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
return ERR_PTR(ret);
}
phys_enc = _enc->base;
-   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
-   phys_enc->intf_idx = p->intf_idx;
+
+   dpu_encoder_phys_init(phys_enc, p);
 
dpu_encoder_phys_cmd_init_ops(_enc->ops);
-   phys_enc->parent = p->parent;
-   phys_enc->dpu_kms = p->dpu_kms;
-   phys_enc->split_role = p->split_role;
phys_enc->intf_mode = INTF_MODE_CMD;
-   phys_enc->enc_spinlock = p->enc_spinlock;
cmd_enc->stream_sel = 0;
-   phys_enc->enable_state = DPU_ENC_DISABLED;
-   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
-   phys_enc->irq[i] = -EINVAL;
 
phys_enc->has_intf_te = test_bit(DPU_INTF_TE,
_enc->dpu_kms->catalog->intf[p->intf_idx - 
INTF_0].features);
 
-   atomic_set(_enc->vblank_refcount, 0);
-   atomic_set(_enc->pending_kickoff_cnt, 0);
-   atomic_set(_enc->pending_ctlstart_cnt, 0);
atomic_set(_enc->pending_vblank_cnt, 0);
-   init_waitqueue_head(_enc->pending_kickoff_wq);
init_waitqueue_head(_enc->pending_vblank_wq);
 
DPU_DEBUG_CMDENC(cmd_enc, "created\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 3a374292f311..dc951fdf473b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -699,7 +699,6 @@ struct 

[Freedreno] [PATCH v3 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs

2023-06-01 Thread Dmitry Baryshkov
Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and
struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and
use them to get the instance index.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 72 ---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 12 ++--
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 18 ++---
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |  2 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   |  8 +--
 5 files changed, 47 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 475b30bef72d..0b9f1b3c6c11 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -339,7 +339,8 @@ void dpu_encoder_helper_report_irq_timeout(struct 
dpu_encoder_phys *phys_enc,
DRM_ERROR("irq timeout id=%u, intf_mode=%s intf=%d wb=%d, pp=%d, 
intr=%d\n",
DRMID(phys_enc->parent),
dpu_encoder_helper_get_intf_type(phys_enc->intf_mode),
-   phys_enc->intf_idx - INTF_0, phys_enc->wb_idx - WB_0,
+   phys_enc->hw_intf ? phys_enc->hw_intf->idx - INTF_0 : 
-1,
+   phys_enc->hw_wb ? phys_enc->hw_wb->idx - WB_0 : -1,
phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
 
dpu_encoder_frame_done_callback(phys_enc->parent, phys_enc,
@@ -1419,7 +1420,8 @@ void dpu_encoder_frame_done_callback(
 */
trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc), 
event,

dpu_encoder_helper_get_intf_type(ready_phys->intf_mode),
-   ready_phys->intf_idx, 
ready_phys->wb_idx);
+   ready_phys->hw_intf ? 
ready_phys->hw_intf->idx : -1,
+   ready_phys->hw_wb ? 
ready_phys->hw_wb->idx : -1);
return;
}
 
@@ -1499,7 +1501,8 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder 
*drm_enc,
 
trace_dpu_enc_trigger_flush(DRMID(drm_enc),
dpu_encoder_helper_get_intf_type(phys->intf_mode),
-   phys->intf_idx, phys->wb_idx,
+   phys->hw_intf ? phys->hw_intf->idx : -1,
+   phys->hw_wb ? phys->hw_wb->idx : -1,
pending_kickoff_cnt, ctl->idx,
extra_flush_bits, ret);
 }
@@ -2110,7 +2113,8 @@ static int _dpu_encoder_status_show(struct seq_file *s, 
void *data)
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
 
seq_printf(s, "intf:%d  wb:%d  vsync:%8d underrun:%8d",
-   phys->intf_idx - INTF_0, phys->wb_idx - WB_0,
+   phys->hw_intf ? phys->hw_intf->idx - INTF_0 : 
-1,
+   phys->hw_wb ? phys->hw_wb->idx - WB_0 : -1,
atomic_read(>vsync_cnt),
atomic_read(>underrun_cnt));
 
@@ -2274,6 +2278,8 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right
 */
u32 controller_id = disp_info->h_tile_instance[i];
+   enum dpu_intf intf_idx;
+   enum dpu_wb wb_idx;
 
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2287,57 +2293,39 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
 
-   phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
+   intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,

disp_info->intf_type,
controller_id);
 
-   phys_params.wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
+   wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
disp_info->intf_type, controller_id);
-   /*
-* The phys_params might represent either an INTF or a WB unit, 
but not
-* both of them at the same time.
-*/
-   if ((phys_params.intf_idx == INTF_MAX) &&
-   (phys_params.wb_idx == WB_MAX)) {
-   DPU_ERROR_ENC(dpu_enc, "could not get intf or wb: type 
%d, id %d\n",
- disp_info->intf_type, 
controller_id);
-   ret = -EINVAL;
-   }
 
-   if ((phys_params.intf_idx != 

[Freedreno] [PATCH v3 4/7] drm/msm/dpu: inline dpu_encoder_get_wb()

2023-06-01 Thread Dmitry Baryshkov
The function dpu_encoder_get_wb() returns controller_id if the
corresponding WB is present in the catalog. We can inline this function
and rely on dpu_rm_get_wb() returning NULL for indices for which the
WB is not present on the device.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++---
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0b9f1b3c6c11..94432451e175 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1288,22 +1288,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct 
dpu_mdss_cfg *catalog,
return INTF_MAX;
 }
 
-static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog,
-   enum dpu_intf_type type, u32 controller_id)
-{
-   int i = 0;
-
-   if (type != INTF_WB)
-   return WB_MAX;
-
-   for (i = 0; i < catalog->wb_count; i++) {
-   if (catalog->wb[i].id == controller_id)
-   return catalog->wb[i].id;
-   }
-
-   return WB_MAX;
-}
-
 void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phy_enc)
 {
@@ -2279,7 +2263,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 */
u32 controller_id = disp_info->h_tile_instance[i];
enum dpu_intf intf_idx;
-   enum dpu_wb wb_idx;
 
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2297,14 +2280,11 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,

disp_info->intf_type,
controller_id);
 
-   wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
-   disp_info->intf_type, controller_id);
-
if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
phys_params.hw_intf = dpu_rm_get_intf(_kms->rm, 
intf_idx);
 
-   if (wb_idx >= WB_0 && wb_idx < WB_MAX)
-   phys_params.hw_wb = dpu_rm_get_wb(_kms->rm, wb_idx);
+   if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX)
+   phys_params.hw_wb = dpu_rm_get_wb(_kms->rm, 
controller_id);
 
if (!phys_params.hw_intf && !phys_params.hw_wb) {
DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at 
idx: %d\n", i);
-- 
2.39.2



[Freedreno] [PATCH v3 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()

2023-06-01 Thread Dmitry Baryshkov
There is little sense to get intf index just to call dpu_rm_get_intf()
on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 94432451e175..c04b551c9d34 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1270,22 +1270,23 @@ static void dpu_encoder_virt_atomic_disable(struct 
drm_encoder *drm_enc,
mutex_unlock(_enc->enc_lock);
 }
 
-static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog,
+static struct dpu_hw_intf *dpu_encoder_get_intf(const struct dpu_mdss_cfg 
*catalog,
+   struct dpu_rm *dpu_rm,
enum dpu_intf_type type, u32 controller_id)
 {
int i = 0;
 
if (type == INTF_WB)
-   return INTF_MAX;
+   return NULL;
 
for (i = 0; i < catalog->intf_count; i++) {
if (catalog->intf[i].type == type
&& catalog->intf[i].controller_id == controller_id) {
-   return catalog->intf[i].id;
+   return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id);
}
}
 
-   return INTF_MAX;
+   return NULL;
 }
 
 void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
@@ -2262,7 +2263,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right
 */
u32 controller_id = disp_info->h_tile_instance[i];
-   enum dpu_intf intf_idx;
 
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2276,12 +2276,9 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
 
-   intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
-   
disp_info->intf_type,
-   controller_id);
-
-   if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
-   phys_params.hw_intf = dpu_rm_get_intf(_kms->rm, 
intf_idx);
+   phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, 
_kms->rm,
+  disp_info->intf_type,
+  controller_id);
 
if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX)
phys_params.hw_wb = dpu_rm_get_wb(_kms->rm, 
controller_id);
@@ -2305,7 +2302,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n");
break;
}
-
}
 
mutex_unlock(_enc->enc_lock);
-- 
2.39.2



[Freedreno] [PATCH v3 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()

2023-06-01 Thread Dmitry Baryshkov
There is no reason to split the dpu_encoder interface into separate
_init() and _setup() phases. Merge them into a single function.

Reviewed-by: Abhinav Kumar 
Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 55 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 -
 3 files changed, 56 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d7cd4734dc7d..d4b759e8f2ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2388,7 +2388,8 @@ static const struct drm_encoder_funcs dpu_encoder_funcs = 
{
.early_unregister = dpu_encoder_early_unregister,
 };
 
-int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+   int drm_enc_mode,
struct msm_display_info *disp_info)
 {
struct msm_drm_private *priv = dev->dev_private;
@@ -2397,7 +2398,23 @@ int dpu_encoder_setup(struct drm_device *dev, struct 
drm_encoder *enc,
struct dpu_encoder_virt *dpu_enc = NULL;
int ret = 0;
 
-   dpu_enc = to_dpu_encoder_virt(enc);
+   dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
+   if (!dpu_enc)
+   return ERR_PTR(-ENOMEM);
+
+   ret = drm_encoder_init(dev, _enc->base, _encoder_funcs,
+  drm_enc_mode, NULL);
+   if (ret) {
+   devm_kfree(dev->dev, dpu_enc);
+   return ERR_PTR(ret);
+   }
+
+   drm_encoder_helper_add(_enc->base, _encoder_helper_funcs);
+
+   spin_lock_init(_enc->enc_spinlock);
+   dpu_enc->enabled = false;
+   mutex_init(_enc->enc_lock);
+   mutex_init(_enc->rc_lock);
 
ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info);
if (ret)
@@ -2426,44 +2443,14 @@ int dpu_encoder_setup(struct drm_device *dev, struct 
drm_encoder *enc,
 
DPU_DEBUG_ENC(dpu_enc, "created\n");
 
-   return ret;
+   return _enc->base;
 
 fail:
DPU_ERROR("failed to create encoder\n");
if (drm_enc)
dpu_encoder_destroy(drm_enc);
 
-   return ret;
-
-
-}
-
-struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
-   int drm_enc_mode)
-{
-   struct dpu_encoder_virt *dpu_enc = NULL;
-   int rc = 0;
-
-   dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
-   if (!dpu_enc)
-   return ERR_PTR(-ENOMEM);
-
-
-   rc = drm_encoder_init(dev, _enc->base, _encoder_funcs,
- drm_enc_mode, NULL);
-   if (rc) {
-   devm_kfree(dev->dev, dpu_enc);
-   return ERR_PTR(rc);
-   }
-
-   drm_encoder_helper_add(_enc->base, _encoder_helper_funcs);
-
-   spin_lock_init(_enc->enc_spinlock);
-   dpu_enc->enabled = false;
-   mutex_init(_enc->enc_lock);
-   mutex_init(_enc->rc_lock);
-
-   return _enc->base;
+   return ERR_PTR(ret);
 }
 
 int dpu_encoder_wait_for_event(struct drm_encoder *drm_enc,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 6d14f84dd43f..90e1925d7770 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -130,20 +130,12 @@ void dpu_encoder_virt_runtime_resume(struct drm_encoder 
*encoder);
 /**
  * dpu_encoder_init - initialize virtual encoder object
  * @dev:Pointer to drm device structure
+ * @drm_enc_mode: corresponding DRM_MODE_ENCODER_* constant
  * @disp_info:  Pointer to display information structure
  * Returns: Pointer to newly created drm encoder
  */
-struct drm_encoder *dpu_encoder_init(
-   struct drm_device *dev,
-   int drm_enc_mode);
-
-/**
- * dpu_encoder_setup - setup dpu_encoder for the display probed
- * @dev:   Pointer to drm device structure
- * @enc:   Pointer to the drm_encoder
- * @disp_info: Pointer to the display info
- */
-int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+   int drm_enc_mode,
struct msm_display_info *disp_info);
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 8ce057cc9374..70a17ef8281f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -535,15 +535,23 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
!msm_dsi_is_master_dsi(priv->dsi[i]))
continue;
 
-   encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+   memset(, 0, sizeof(info));
+   

[Freedreno] [PATCH v3 0/7] drm/msm/dpu: simplify DPU encoder init

2023-06-01 Thread Dmitry Baryshkov
Rework dpu_encoder initialization code, simplifying calling sequences
and separating common init parts.

Changes since v2:
- Rebased on top of msm-next-lumag branch

Changes since v1:
- Withdrawn two pathes for a later consideration
- Changed dpu_encoder_phys_init() to return void (Abhinav)
- Added small simplifications of dpu_encoder_phys_cmd_init() and
  dpu_encoder_phys_wb_init()


Dmitry Baryshkov (7):
  drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
  drm/msm/dpu: separate common function to init physical encoder
  drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
  drm/msm/dpu: inline dpu_encoder_get_wb()
  drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
  drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
  drm/msm/dpu: simplify dpu_encoder_phys_wb_init()

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 178 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h   |  14 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  15 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |  37 ++--
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |  19 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   |  35 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  87 -
 7 files changed, 141 insertions(+), 244 deletions(-)

-- 
2.39.2



[Freedreno] [PATCH v3 7/7] ARM: dts: qcom: msm8226: Add mdss nodes

2023-06-01 Thread Luca Weiss
Add the nodes that describe the mdss so that display can work on
MSM8226.

Signed-off-by: Luca Weiss 
---
 arch/arm/boot/dts/qcom-msm8226.dtsi | 127 
 1 file changed, 127 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi 
b/arch/arm/boot/dts/qcom-msm8226.dtsi
index 42acb9ddb8cc..9f53747d2990 100644
--- a/arch/arm/boot/dts/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
@@ -636,6 +636,133 @@ smd-edge {
label = "lpass";
};
};
+
+   mdss: display-subsystem@fd90 {
+   compatible = "qcom,mdss";
+   reg = <0xfd90 0x100>, <0xfd924000 0x1000>;
+   reg-names = "mdss_phys", "vbif_phys";
+
+   power-domains = < MDSS_GDSC>;
+
+   clocks = < MDSS_AHB_CLK>,
+< MDSS_AXI_CLK>,
+< MDSS_VSYNC_CLK>;
+   clock-names = "iface",
+ "bus",
+ "vsync";
+
+   interrupts = ;
+
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   status = "disabled";
+
+   mdss_mdp: display-controller@fd90 {
+   compatible = "qcom,msm8226-mdp5", "qcom,mdp5";
+   reg = <0xfd900100 0x22000>;
+   reg-names = "mdp_phys";
+
+   interrupt-parent = <>;
+   interrupts = <0>;
+
+   clocks = < MDSS_AHB_CLK>,
+< MDSS_AXI_CLK>,
+< MDSS_MDP_CLK>,
+< MDSS_VSYNC_CLK>;
+   clock-names = "iface",
+ "bus",
+ "core",
+ "vsync";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   mdss_mdp_intf1_out: endpoint {
+   remote-endpoint = 
<_dsi0_in>;
+   };
+   };
+   };
+   };
+
+   mdss_dsi0: dsi@fd922800 {
+   compatible = "qcom,msm8226-dsi-ctrl",
+"qcom,mdss-dsi-ctrl";
+   reg = <0xfd922800 0x1f8>;
+   reg-names = "dsi_ctrl";
+
+   interrupt-parent = <>;
+   interrupts = <4>;
+
+   assigned-clocks = < BYTE0_CLK_SRC>,
+ < PCLK0_CLK_SRC>;
+   assigned-clock-parents = <_dsi0_phy 0>,
+<_dsi0_phy 1>;
+
+   clocks = < MDSS_MDP_CLK>,
+< MDSS_AHB_CLK>,
+< MDSS_AXI_CLK>,
+< MDSS_BYTE0_CLK>,
+< MDSS_PCLK0_CLK>,
+< MDSS_ESC0_CLK>,
+< MMSS_MISC_AHB_CLK>;
+   clock-names = "mdp_core",
+ "iface",
+ "bus",
+ "byte",
+ "pixel",
+ "core",
+ "core_mmss";
+
+   phys = <_dsi0_phy>;
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   mdss_dsi0_in: endpoint {
+   remote-endpoint = 
<_mdp_intf1_out>;
+  

[Freedreno] [PATCH v3 0/7] Display support for MSM8226

2023-06-01 Thread Luca Weiss
This series adds the required configs for MDP5 and DSI blocks that are
needed for MDSS on MSM8226. Finally we can add the new nodes into the
dts.

Tested on apq8026-lg-lenok and msm8926-htc-memul.

Signed-off-by: Luca Weiss 
---
Changes in v3:
- Adjust mdss labels to new style (Stephan)
- Link to v2: 
https://lore.kernel.org/r/20230308-msm8226-mdp-v2-0-e005b769e...@z3ntu.xyz

Changes in v2:
- In dsi-phy-28nm.yaml fix the order of the compatibles 1/7 (Conor)
- Remove leftover debugging comments from 6/7 (Konrad)
- Rewrap some clock-names lines and move status property last in 7/7
  (Konrad)
- Pick up tags
- Link to v1: 
https://lore.kernel.org/r/20230308-msm8226-mdp-v1-0-679f335d3...@z3ntu.xyz

---
Luca Weiss (7):
  dt-bindings: msm: dsi-phy-28nm: Document msm8226 compatible
  dt-bindings: display/msm: dsi-controller-main: Add msm8226 compatible
  dt-bindings: display/msm: qcom,mdp5: Add msm8226 compatible
  drm/msm/mdp5: Add MDP5 configuration for MSM8226
  drm/msm/dsi: Add configuration for MSM8226
  drm/msm/dsi: Add phy configuration for MSM8226
  ARM: dts: qcom: msm8226: Add mdss nodes

 .../bindings/display/msm/dsi-controller-main.yaml  |   2 +
 .../bindings/display/msm/dsi-phy-28nm.yaml |   3 +-
 .../devicetree/bindings/display/msm/qcom,mdp5.yaml |   1 +
 .../devicetree/bindings/display/msm/qcom,mdss.yaml |   1 +
 arch/arm/boot/dts/qcom-msm8226.dtsi| 127 +
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c   |  82 +
 drivers/gpu/drm/msm/dsi/dsi_cfg.c  |   2 +
 drivers/gpu/drm/msm/dsi/dsi_cfg.h  |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c  |   2 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h  |   3 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c |  97 
 11 files changed, 319 insertions(+), 2 deletions(-)
---
base-commit: 1b3183710d69a48baf728cc1bee9f1fb3cfeb507
change-id: 20230308-msm8226-mdp-6431e8d672a0

Best regards,
-- 
Luca Weiss 



[Freedreno] [PATCH v3 1/7] dt-bindings: msm: dsi-phy-28nm: Document msm8226 compatible

2023-06-01 Thread Luca Weiss
The MSM8226 SoC uses a slightly different 28nm dsi phy. Add a new
compatible for it.

And while we're at it, in the dsi-phy-28nm.yaml move the 8960 compatible
to its correct place so its sorted alphabetically.

Acked-by: Conor Dooley 
Signed-off-by: Luca Weiss 
---
 Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml | 3 ++-
 Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml| 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
index cf4a338c4661..62fb3e484eb2 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
@@ -15,10 +15,11 @@ allOf:
 properties:
   compatible:
 enum:
+  - qcom,dsi-phy-28nm-8226
+  - qcom,dsi-phy-28nm-8960
   - qcom,dsi-phy-28nm-hpm
   - qcom,dsi-phy-28nm-hpm-fam-b
   - qcom,dsi-phy-28nm-lp
-  - qcom,dsi-phy-28nm-8960
 
   reg:
 items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml 
b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
index b0100105e428..db9f07c6142d 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml
@@ -125,6 +125,7 @@ patternProperties:
   - qcom,dsi-phy-14nm-660
   - qcom,dsi-phy-14nm-8953
   - qcom,dsi-phy-20nm
+  - qcom,dsi-phy-28nm-8226
   - qcom,dsi-phy-28nm-hpm
   - qcom,dsi-phy-28nm-lp
   - qcom,hdmi-phy-8084

-- 
2.40.1



[Freedreno] [PATCH v3 5/7] drm/msm/dsi: Add configuration for MSM8226

2023-06-01 Thread Luca Weiss
Add the config for the v1.0.2 DSI found on MSM8226. We can reuse
existing bits from other revisions that are identical for v1.0.2.

Reviewed-by: Dmitry Baryshkov 
Reviewed-by: Konrad Dybcio 
Signed-off-by: Luca Weiss 
---
 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 2 ++
 drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c 
b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 29ccd755cc2e..8a5fb6df7210 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -245,6 +245,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] 
= {
_dsi_cfg, _dsi_v2_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0,
_apq8084_dsi_cfg, _dsi_6g_host_ops},
+   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0_2,
+   _apq8084_dsi_cfg, _dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1,
_apq8084_dsi_cfg, _dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1_1,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h 
b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 91bdaf50bb1a..43f0dd74edb6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -11,6 +11,7 @@
 #define MSM_DSI_VER_MAJOR_V2   0x02
 #define MSM_DSI_VER_MAJOR_6G   0x03
 #define MSM_DSI_6G_VER_MINOR_V1_0  0x1000
+#define MSM_DSI_6G_VER_MINOR_V1_0_20x1002
 #define MSM_DSI_6G_VER_MINOR_V1_1  0x1001
 #define MSM_DSI_6G_VER_MINOR_V1_1_10x10010001
 #define MSM_DSI_6G_VER_MINOR_V1_2  0x1002

-- 
2.40.1



[Freedreno] [PATCH v3 6/7] drm/msm/dsi: Add phy configuration for MSM8226

2023-06-01 Thread Luca Weiss
MSM8226 uses a modified PLL lock sequence compared to MSM8974, which is
based on the function dsi_pll_enable_seq_m in the msm-3.10 kernel.

Worth noting that the msm-3.10 downstream kernel also will try other
sequences in case this one doesn't work, but during testing it has shown
that the _m sequence succeeds first time also:

  .pll_enable_seqs[0] = dsi_pll_enable_seq_m,
  .pll_enable_seqs[1] = dsi_pll_enable_seq_m,
  .pll_enable_seqs[2] = dsi_pll_enable_seq_d,
  .pll_enable_seqs[3] = dsi_pll_enable_seq_d,
  .pll_enable_seqs[4] = dsi_pll_enable_seq_f1,
  .pll_enable_seqs[5] = dsi_pll_enable_seq_c,
  .pll_enable_seqs[6] = dsi_pll_enable_seq_e,

We may need to expand this in the future.

Signed-off-by: Luca Weiss 
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c  |  2 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h  |  3 +-
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 97 ++
 3 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index bb09cbe8ff86..9d5795c58a98 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -541,6 +541,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
  .data = _phy_28nm_hpm_famb_cfgs },
{ .compatible = "qcom,dsi-phy-28nm-lp",
  .data = _phy_28nm_lp_cfgs },
+   { .compatible = "qcom,dsi-phy-28nm-8226",
+ .data = _phy_28nm_8226_cfgs },
 #endif
 #ifdef CONFIG_DRM_MSM_DSI_20NM_PHY
{ .compatible = "qcom,dsi-phy-20nm",
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 7137a17ae523..8b640d174785 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -46,8 +46,9 @@ struct msm_dsi_phy_cfg {
 extern const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_famb_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs;
-extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8226_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index 4c1bf55c5f38..ceec7bb87bf1 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -37,6 +37,7 @@
 
 /* v2.0.0 28nm LP implementation */
 #define DSI_PHY_28NM_QUIRK_PHY_LP  BIT(0)
+#define DSI_PHY_28NM_QUIRK_PHY_8226BIT(1)
 
 #define LPFR_LUT_SIZE  10
 struct lpfr_cfg {
@@ -377,6 +378,74 @@ static int dsi_pll_28nm_vco_prepare_hpm(struct clk_hw *hw)
return ret;
 }
 
+static int dsi_pll_28nm_vco_prepare_8226(struct clk_hw *hw)
+{
+   struct dsi_pll_28nm *pll_28nm = to_pll_28nm(hw);
+   struct device *dev = _28nm->phy->pdev->dev;
+   void __iomem *base = pll_28nm->phy->pll_base;
+   u32 max_reads = 5, timeout_us = 100;
+   bool locked;
+   u32 val;
+   int i;
+
+   DBG("id=%d", pll_28nm->phy->id);
+
+   pll_28nm_software_reset(pll_28nm);
+
+   /*
+* PLL power up sequence.
+* Add necessary delays recommended by hardware.
+*/
+   dsi_phy_write(base + REG_DSI_28nm_PHY_PLL_CAL_CFG1, 0x34);
+
+   val = DSI_28nm_PHY_PLL_GLB_CFG_PLL_PWRDN_B;
+   dsi_phy_write_udelay(base + REG_DSI_28nm_PHY_PLL_GLB_CFG, val, 200);
+
+   val |= DSI_28nm_PHY_PLL_GLB_CFG_PLL_PWRGEN_PWRDN_B;
+   dsi_phy_write_udelay(base + REG_DSI_28nm_PHY_PLL_GLB_CFG, val, 200);
+
+   val |= DSI_28nm_PHY_PLL_GLB_CFG_PLL_LDO_PWRDN_B;
+   val |= DSI_28nm_PHY_PLL_GLB_CFG_PLL_ENABLE;
+   dsi_phy_write_udelay(base + REG_DSI_28nm_PHY_PLL_GLB_CFG, val, 600);
+
+   for (i = 0; i < 7; i++) {
+   /* DSI Uniphy lock detect setting */
+   dsi_phy_write(base + REG_DSI_28nm_PHY_PLL_LKDET_CFG2, 0x0d);
+   dsi_phy_write_udelay(base + REG_DSI_28nm_PHY_PLL_LKDET_CFG2,
+   0x0c, 100);
+   dsi_phy_write(base + REG_DSI_28nm_PHY_PLL_LKDET_CFG2, 0x0d);
+
+   /* poll for PLL ready status */
+   locked = pll_28nm_poll_for_ready(pll_28nm,
+   max_reads, timeout_us);
+   if (locked)
+   break;
+
+   pll_28nm_software_reset(pll_28nm);
+
+   /*
+* PLL power up sequence.
+* Add necessary delays recommended by hardware.
+*/
+   dsi_phy_write_udelay(base + REG_DSI_28nm_PHY_PLL_PWRGEN_CFG, 
0x00, 50);
+
+   val = DSI_28nm_PHY_PLL_GLB_CFG_PLL_PWRDN_B;

[Freedreno] [PATCH v3 3/7] dt-bindings: display/msm: qcom, mdp5: Add msm8226 compatible

2023-06-01 Thread Luca Weiss
Add the compatible for the MDP5 found on MSM8226.

Acked-by: Conor Dooley 
Signed-off-by: Luca Weiss 
---
 Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml 
b/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml
index a763cf8da122..2fe032d0e8f8 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,mdp5.yaml
@@ -22,6 +22,7 @@ properties:
   - items:
   - enum:
   - qcom,apq8084-mdp5
+  - qcom,msm8226-mdp5
   - qcom,msm8916-mdp5
   - qcom,msm8917-mdp5
   - qcom,msm8953-mdp5

-- 
2.40.1



[Freedreno] [PATCH v3 2/7] dt-bindings: display/msm: dsi-controller-main: Add msm8226 compatible

2023-06-01 Thread Luca Weiss
Add the compatible for the DSI found on MSM8226.

Acked-by: Conor Dooley 
Signed-off-by: Luca Weiss 
---
 Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 130e16d025bc..660e0f496826 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -15,6 +15,7 @@ properties:
   - items:
   - enum:
   - qcom,apq8064-dsi-ctrl
+  - qcom,msm8226-dsi-ctrl
   - qcom,msm8916-dsi-ctrl
   - qcom,msm8953-dsi-ctrl
   - qcom,msm8974-dsi-ctrl
@@ -256,6 +257,7 @@ allOf:
 compatible:
   contains:
 enum:
+  - qcom,msm8226-dsi-ctrl
   - qcom,msm8974-dsi-ctrl
 then:
   properties:

-- 
2.40.1



[Freedreno] [PATCH v3 4/7] drm/msm/mdp5: Add MDP5 configuration for MSM8226

2023-06-01 Thread Luca Weiss
Add the required config for the v1.1 MDP5 found on MSM8226.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Luca Weiss 
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c | 82 
 1 file changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 2eec2d78f32a..694d54341337 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -103,6 +103,87 @@ static const struct mdp5_cfg_hw msm8x74v1_config = {
.max_clk = 2,
 };
 
+static const struct mdp5_cfg_hw msm8x26_config = {
+   .name = "msm8x26",
+   .mdp = {
+   .count = 1,
+   .caps = MDP_CAP_SMP |
+   0,
+   },
+   .smp = {
+   .mmb_count = 7,
+   .mmb_size = 4096,
+   .clients = {
+   [SSPP_VIG0] =  1,
+   [SSPP_DMA0] = 4,
+   [SSPP_RGB0] = 7,
+   },
+   },
+   .ctl = {
+   .count = 2,
+   .base = { 0x00500, 0x00600 },
+   .flush_hw_mask = 0x0003,
+   },
+   .pipe_vig = {
+   .count = 1,
+   .base = { 0x01100 },
+   .caps = MDP_PIPE_CAP_HFLIP |
+   MDP_PIPE_CAP_VFLIP |
+   MDP_PIPE_CAP_SCALE |
+   MDP_PIPE_CAP_CSC   |
+   0,
+   },
+   .pipe_rgb = {
+   .count = 1,
+   .base = { 0x01d00 },
+   .caps = MDP_PIPE_CAP_HFLIP |
+   MDP_PIPE_CAP_VFLIP |
+   MDP_PIPE_CAP_SCALE |
+   0,
+   },
+   .pipe_dma = {
+   .count = 1,
+   .base = { 0x02900 },
+   .caps = MDP_PIPE_CAP_HFLIP |
+   MDP_PIPE_CAP_VFLIP |
+   0,
+   },
+   .lm = {
+   .count = 2,
+   .base = { 0x03100, 0x03d00 },
+   .instances = {
+   { .id = 0, .pp = 0, .dspp = 0,
+ .caps = MDP_LM_CAP_DISPLAY, },
+   { .id = 1, .pp = -1, .dspp = -1,
+ .caps = MDP_LM_CAP_WB },
+},
+   .nb_stages = 2,
+   .max_width = 2048,
+   .max_height = 0x,
+   },
+   .dspp = {
+   .count = 1,
+   .base = { 0x04500 },
+   },
+   .pp = {
+   .count = 1,
+   .base = { 0x21a00 },
+   },
+   .intf = {
+   .base = { 0x0, 0x21200 },
+   .connect = {
+   [0] = INTF_DISABLED,
+   [1] = INTF_DSI,
+   },
+   },
+   .perf = {
+   .ab_inefficiency = 100,
+   .ib_inefficiency = 200,
+   .clk_inefficiency = 125
+   },
+   .max_clk = 2,
+};
+
 static const struct mdp5_cfg_hw msm8x74v2_config = {
.name = "msm8x74",
.mdp = {
@@ -1236,6 +1317,7 @@ static const struct mdp5_cfg_hw sdm660_config = {
 
 static const struct mdp5_cfg_handler cfg_handlers_v1[] = {
{ .revision = 0, .config = { .hw = _config } },
+   { .revision = 1, .config = { .hw = _config } },
{ .revision = 2, .config = { .hw = _config } },
{ .revision = 3, .config = { .hw = _config } },
{ .revision = 6, .config = { .hw = _config } },

-- 
2.40.1



Re: [Freedreno] [PATCH v2] drm/msm/dpu: clean up dpu_kms_get_clk_rate() returns

2023-06-01 Thread Abhinav Kumar




On 5/26/2023 4:51 AM, Dan Carpenter wrote:

Static analysis tools complain about the -EINVAL error code being
stored in an unsigned variable.  Let's change this to match
the clk_get_rate() function which is type unsigned long and returns
zero on error.

Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dan Carpenter 
---
v2: In v1 I change the type to int which was wrong.  This is a different
 approach.  CC the freedreno list this time too.

  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++--
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e..25e6a15eaf9f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -979,13 +979,13 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
return 0;
  }
  
-u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name)

+unsigned long dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name)
  {
struct clk *clk;
  
  	clk = msm_clk_bulk_get_clock(dpu_kms->clocks, dpu_kms->num_clocks, clock_name);

if (!clk)
-   return -EINVAL;
+   return 0;
  


Currently, the only caller of this API is 
dpu_encoder_phys_cmd_tearcheck_config which seems to handle <=0 so this 
change should be fine. Hence



Reviewed-by: Abhinav Kumar 


Re: [Freedreno] [PATCH] drm/msm: Remove unnecessary (void*) conversions

2023-06-01 Thread Abhinav Kumar




On 5/21/2023 6:32 PM, Su Hui wrote:

Pointer variables of (void*) type do not require type cast.

Signed-off-by: Su Hui 
---


Reviewed-by: Abhinav Kumar 



Re: [Freedreno] [PATCH v5 11/13] drm/msm: Use regular fbdev I/O helpers

2023-06-01 Thread Abhinav Kumar




On 5/30/2023 8:02 AM, Thomas Zimmermann wrote:

Use the regular fbdev helpers for framebuffer I/O instead of DRM's
helpers. Msm does not use damage handling, so DRM's fbdev helpers
are mere wrappers around the fbdev code.

By using fbdev helpers directly within each DRM fbdev emulation,
we can eventually remove DRM's wrapper functions entirely.

Msm's fbdev emulation has been incomplete as it didn't implement
damage handling. Partilly fix this by implementing damage handling
for write and draw operation. It is still missing for mmaped pages.

v4:
* use initializer macros for struct fb_ops
* partially support damage handling
v2:
* use FB_SYS_HELPERS option

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Dmitry Baryshkov 
Acked-by: Sam Ravnborg 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
---


Reviewed-by: Abhinav Kumar 


Re: [Freedreno] [PATCH 00/53] drm: Convert to platform remove callback returning void

2023-06-01 Thread Uwe Kleine-König
Hello,

On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote:
> this patch series adapts the platform drivers below drivers/gpu/drm
> to use the .remove_new() callback. Compared to the traditional .remove()
> callback .remove_new() returns no value. This is a good thing because
> the driver core doesn't (and cannot) cope for errors during remove. The
> only effect of a non-zero return value in .remove() is that the driver
> core emits a warning. The device is removed anyhow and an early return
> from .remove() usually yields a resource leak.
> 
> By changing the remove callback to return void driver authors cannot
> reasonably (but wrongly) assume any more that there happens some kind of
> cleanup later.

I wonder if someone would volunteer to add the whole series to
drm-misc-next?!

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature


[Freedreno] [PATCH] drm/msm: Remove unnecessary (void*) conversions

2023-06-01 Thread Su Hui
Pointer variables of (void*) type do not require type cast.

Signed-off-by: Su Hui 
---
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 2 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c  | 2 +-
 drivers/gpu/drm/msm/msm_debugfs.c | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c 
b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
index 6bd397a85834..169b8fe688f8 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
@@ -69,7 +69,7 @@ static void roq_print(struct msm_gpu *gpu, struct drm_printer 
*p)
 
 static int show(struct seq_file *m, void *arg)
 {
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
struct drm_printer p = drm_seq_file_printer(m);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index cc66ddffe672..6e684a7b49a1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1392,7 +1392,7 @@ DEFINE_SHOW_ATTRIBUTE(_dpu_debugfs_status);
 
 static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 {
-   struct drm_crtc *crtc = (struct drm_crtc *) s->private;
+   struct drm_crtc *crtc = s->private;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
 
seq_printf(s, "client type: %d\n", dpu_crtc_get_client_type(crtc));
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e..3b307ce637a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -57,8 +57,8 @@ static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
 static int _dpu_danger_signal_status(struct seq_file *s,
bool danger_status)
 {
-   struct dpu_kms *kms = (struct dpu_kms *)s->private;
struct dpu_danger_safe_status status;
+   struct dpu_kms *kms = s->private;
int i;
 
if (!kms->hw_mdp) {
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c 
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 29ae5c9613f3..323079cfd698 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -229,7 +229,7 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
 #ifdef CONFIG_DEBUG_FS
 static int smp_show(struct seq_file *m, void *arg)
 {
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c 
b/drivers/gpu/drm/msm/msm_debugfs.c
index 9c0e633a3a61..a0a936f80ae3 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -211,7 +211,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(shrink_fops,
 
 static int msm_gem_show(struct seq_file *m, void *arg)
 {
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct msm_drm_private *priv = dev->dev_private;
int ret;
@@ -229,7 +229,7 @@ static int msm_gem_show(struct seq_file *m, void *arg)
 
 static int msm_mm_show(struct seq_file *m, void *arg)
 {
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_printer p = drm_seq_file_printer(m);
 
@@ -240,7 +240,7 @@ static int msm_mm_show(struct seq_file *m, void *arg)
 
 static int msm_fb_show(struct seq_file *m, void *arg)
 {
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_framebuffer *fb, *fbdev_fb = NULL;
 
-- 
2.30.2



Re: [Freedreno] [PATCH v5 08/12] drm/msm/dpu: Add SM6375 support

2023-06-01 Thread Marijn Suijten
On 2023-05-23 09:46:19, Konrad Dybcio wrote:
> Add basic SM6375 support to the DPU1 driver to enable display output.

Nit: The SM6350 commit doesn't use the word "basic" here: what does it
mean?  Is this addition not complete (because it seems so)?

> Reviewed-by: Dmitry Baryshkov 
> Signed-off-by: Konrad Dybcio 
> ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h | 139 
> +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |   1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |   1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|   1 +
>  4 files changed, 142 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h
> new file mode 100644
> index ..924f2526c06a
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_9_sm6375.h
> @@ -0,0 +1,139 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef _DPU_6_9_SM6375_H
> +#define _DPU_6_9_SM6375_H
> +
> +static const struct dpu_caps sm6375_dpu_caps = {
> + .max_mixer_width = DEFAULT_DPU_LINE_WIDTH,
> + .max_mixer_blendstages = 0x4,
> + .qseed_type = DPU_SSPP_SCALER_QSEED4,
> + .has_dim_layer = true,
> + .has_idle_pc = true,
> + .max_linewidth = 2160,
> + .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> +};
> +
> +static const struct dpu_ubwc_cfg sm6375_ubwc_cfg = {
> + .ubwc_version = DPU_HW_UBWC_VER_20,
> + .ubwc_swizzle = 6,
> + .highest_bank_bit = 1,
> +};
> +
> +static const struct dpu_mdp_cfg sm6375_mdp[] = {
> + {
> + .name = "top_0", .id = MDP_TOP,
> + .base = 0x0, .len = 0x494,
> + .features = 0,
> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0 },
> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
> + },
> +};
> +
> +static const struct dpu_ctl_cfg sm6375_ctl[] = {
> + {
> + .name = "ctl_0", .id = CTL_0,
> + .base = 0x1000, .len = 0x1dc,
> + .features = BIT(DPU_CTL_ACTIVE_CFG),
> + .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> + },
> +};
> +
> +static const struct dpu_sspp_cfg sm6375_sspp[] = {
> + SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f8, VIG_SC7180_MASK,
> + sm6115_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> + SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f8, DMA_SDM845_MASK,
> + sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> +};
> +
> +static const struct dpu_lm_cfg sm6375_lm[] = {
> + LM_BLK("lm_0", LM_0, 0x44000, MIXER_QCM2290_MASK,
> + _lm_sblk, PINGPONG_0, 0, DSPP_0),
> +};
> +
> +static const struct dpu_dspp_cfg sm6375_dspp[] = {
> + DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
> + _dspp_sblk),
> +};
> +
> +static const struct dpu_pingpong_cfg sm6375_pp[] = {
> + PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SM8150_MASK, 0, 
> sdm845_pp_sblk,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + -1),
> +};
> +
> +static const struct dpu_dsc_cfg sm6375_dsc[] = {
> + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)),
> +};
> +
> +static const struct dpu_intf_cfg sm6375_intf[] = {
> + INTF_BLK("intf_0", INTF_0, 0x0, 0x280, INTF_NONE, 0, 0, 0, 0, 0),
> + INTF_BLK_DSI_TE("intf_1", INTF_1, 0x6a800, 0x2c0, INTF_DSI, 0, 24, 
> INTF_SC7280_MASK,

Did you forget to set this back to SC7180?  This will be enabling
DPU_INTF_DATA_COMPRESS otherwise, which is a DPU 7.x feature.

- Marijn

> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27),
> + DPU_IRQ_IDX(MDP_INTF1_TEAR_INTR, 2)),
> +};
> +
> +static const struct dpu_perf_cfg sm6375_perf_data = {
> + .max_bw_low = 520,
> + .max_bw_high = 620,
> + .min_core_ib = 250,
> + .min_llcc_ib = 0, /* No LLCC on this SoC */
> + .min_dram_ib = 160,
> + .min_prefill_lines = 24,
> + /* TODO: confirm danger_lut_tbl */
> + .danger_lut_tbl = {0x, 0x, 0x0},
> + .safe_lut_tbl = {0xfe00, 0xfe00, 0x},
> + .qos_lut_tbl = {
> + {.nentry = ARRAY_SIZE(sm6350_qos_linear_macrotile),
> + .entries = sm6350_qos_linear_macrotile
> + },
> + {.nentry = ARRAY_SIZE(sm6350_qos_linear_macrotile),
> + .entries = sm6350_qos_linear_macrotile
> + },
> + {.nentry = ARRAY_SIZE(sc7180_qos_nrt),
> + .entries = sc7180_qos_nrt
> + },
> + },
> + .cdp_cfg = {
> + {.rd_enable = 1, .wr_enable = 1},
> + {.rd_enable = 1, .wr_enable = 0}
> + },
> + .clk_inefficiency_factor = 105,
> + .bw_inefficiency_factor = 120,
> +};
> +
> +const 

Re: [Freedreno] [PATCH v2 2/3] arm64: dts: qcom: sm8550: fix low_svs RPMhPD labels

2023-06-01 Thread Konrad Dybcio



On 1.06.2023 12:09, Neil Armstrong wrote:
> "low" was written "lov", fix this.
I "lov" making typos.. Thanks for spotting this!

Reviewed-by: Konrad Dybcio 

Konrad
> 
> Fixes: 99d33ee61cb0 ("arm64: dts: qcom: sm8550: Add missing RPMhPD OPP 
> levels")
> Signed-off-by: Neil Armstrong 
> ---
>  arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi 
> b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index 75cd374943eb..972df1ef86ee 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -3649,15 +3649,15 @@ rpmhpd_opp_min_svs: opp-48 {
>   opp-level = 
> ;
>   };
>  
> - rpmhpd_opp_lov_svs_d2: opp-52 {
> + rpmhpd_opp_low_svs_d2: opp-52 {
>   opp-level = 
> ;
>   };
>  
> - rpmhpd_opp_lov_svs_d1: opp-56 {
> + rpmhpd_opp_low_svs_d1: opp-56 {
>   opp-level = 
> ;
>   };
>  
> - rpmhpd_opp_lov_svs_d0: opp-60 {
> + rpmhpd_opp_low_svs_d0: opp-60 {
>   opp-level = 
> ;
>   };
>  
> 


[Freedreno] [PATCH v2 2/3] arm64: dts: qcom: sm8550: fix low_svs RPMhPD labels

2023-06-01 Thread Neil Armstrong
"low" was written "lov", fix this.

Fixes: 99d33ee61cb0 ("arm64: dts: qcom: sm8550: Add missing RPMhPD OPP levels")
Signed-off-by: Neil Armstrong 
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi 
b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 75cd374943eb..972df1ef86ee 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3649,15 +3649,15 @@ rpmhpd_opp_min_svs: opp-48 {
opp-level = 
;
};
 
-   rpmhpd_opp_lov_svs_d2: opp-52 {
+   rpmhpd_opp_low_svs_d2: opp-52 {
opp-level = 
;
};
 
-   rpmhpd_opp_lov_svs_d1: opp-56 {
+   rpmhpd_opp_low_svs_d1: opp-56 {
opp-level = 
;
};
 
-   rpmhpd_opp_lov_svs_d0: opp-60 {
+   rpmhpd_opp_low_svs_d0: opp-60 {
opp-level = 
;
};
 

-- 
2.34.1



[Freedreno] [PATCH v2 1/3] dt-bindings: display: msm: dp-controller: document SM8550 compatible

2023-06-01 Thread Neil Armstrong
The SM8550 & SM8350 SoC shares the same DP TX IP version, use the
SM8350 compatible as fallback for SM8550.

Acked-by: Krzysztof Kozlowski 
Signed-off-by: Neil Armstrong 
---
 Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml 
b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index f0c2237d5f82..7a7cf3fb3e6d 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -29,6 +29,7 @@ properties:
   - items:
   - enum:
   - qcom,sm8450-dp
+  - qcom,sm8550-dp
   - const: qcom,sm8350-dp
 
   reg:

-- 
2.34.1



[Freedreno] [PATCH v2 3/3] arm64: dts: qcom: sm8550: add display port nodes

2023-06-01 Thread Neil Armstrong
Add the Display Port controller subnode to the MDSS node.

Reviewed-by: Konrad Dybcio 
Signed-off-by: Neil Armstrong 
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 89 +++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi 
b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 972df1ef86ee..b41b3981b3ce 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2495,6 +2495,13 @@ dpu_intf2_out: endpoint {
remote-endpoint = 
<_dsi1_in>;
};
};
+
+   port@2 {
+   reg = <2>;
+   dpu_intf0_out: endpoint {
+   remote-endpoint = 
<_dp0_in>;
+   };
+   };
};
 
mdp_opp_table: opp-table {
@@ -2522,6 +2529,84 @@ opp-51400 {
};
};
 
+   mdss_dp0: displayport-controller@ae9 {
+   compatible = "qcom,sm8550-dp", "qcom,sm8350-dp";
+   reg = <0 0xae9 0 0x200>,
+ <0 0xae90200 0 0x200>,
+ <0 0xae90400 0 0xc00>,
+ <0 0xae91000 0 0x400>,
+ <0 0xae91400 0 0x400>;
+   interrupt-parent = <>;
+   interrupts = <12>;
+   clocks = < DISP_CC_MDSS_AHB_CLK>,
+< DISP_CC_MDSS_DPTX0_AUX_CLK>,
+< DISP_CC_MDSS_DPTX0_LINK_CLK>,
+< 
DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
+< 
DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
+   clock-names = "core_iface",
+ "core_aux",
+ "ctrl_link",
+ "ctrl_link_iface",
+ "stream_pixel";
+
+   assigned-clocks = < 
DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
+ < 
DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
+   assigned-clock-parents = <_dp_qmpphy 
QMP_USB43DP_DP_LINK_CLK>,
+<_dp_qmpphy 
QMP_USB43DP_DP_VCO_DIV_CLK>;
+
+   phys = <_dp_qmpphy QMP_USB43DP_DP_PHY>;
+   phy-names = "dp";
+
+   #sound-dai-cells = <0>;
+
+   operating-points-v2 = <_opp_table>;
+   power-domains = < SM8550_MMCX>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   mdss_dp0_in: endpoint {
+   remote-endpoint = 
<_intf0_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   mdss_dp0_out: endpoint {
+   };
+   };
+   };
+
+   dp_opp_table: opp-table {
+   compatible = "operating-points-v2";
+
+   opp-16200 {
+   opp-hz = /bits/ 64 <16200>;
+   required-opps = 
<_opp_low_svs_d1>;
+   };
+
+   opp-27000 {
+   opp-hz = /bits/ 64 <27000>;
+   required-opps = 
<_opp_low_svs>;
+   };
+
+   opp-54000 {
+   opp-hz = /bits/ 64 <54000>;
+   required-opps = 
<_opp_svs_l1>;
+   };

[Freedreno] [PATCH v2 0/3] arm64: dts: qcom: add DP Controller to SM8550 DTS

2023-06-01 Thread Neil Armstrong
The DP output is shared with the USB3 SuperSpeed lanes and is
usually connected to an USB-C port which Altmode is controlled
by the PMIC Glink infrastructure.

DT changes tying the DP controller to the USB-C port on the QRD
board will be sent later.

Signed-off-by: Neil Armstrong 
---
Changes in v2:
- Added review tags
- s/lov_svs/low_svs/
- Applied fixes suggested from Konrad
- Link to v1: 
https://lore.kernel.org/r/20230601-topic-sm8550-upstream-dp-v1-0-29efe2689...@linaro.org

---
Neil Armstrong (3):
  dt-bindings: display: msm: dp-controller: document SM8550 compatible
  arm64: dts: qcom: sm8550: fix low_svs RPMhPD labels
  arm64: dts: qcom: sm8550: add display port nodes

 .../bindings/display/msm/dp-controller.yaml|  1 +
 arch/arm64/boot/dts/qcom/sm8550.dtsi   | 95 --
 2 files changed, 91 insertions(+), 5 deletions(-)
---
base-commit: d4cee89031c80066ec461bb77b5e13a4f37d5fd2
change-id: 20230601-topic-sm8550-upstream-dp-b713ba275d7c

Best regards,
-- 
Neil Armstrong 



Re: [Freedreno] [PATCH 1/2] dt-bindings: display: msm: dp-controller: document SM8550 compatible

2023-06-01 Thread Krzysztof Kozlowski
On 01/06/2023 11:52, Neil Armstrong wrote:
> The SM8550 & SM8350 SoC shares the same DP TX IP version, use the
> SM8350 compatible as fallback for SM8550.
> 
> Signed-off-by: Neil Armstrong 
> ---


Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof



Re: [Freedreno] [PATCH 2/2] arm64: dts: qcom: sm8550: add display port nodes

2023-06-01 Thread Konrad Dybcio



On 1.06.2023 11:52, Neil Armstrong wrote:
> Add the Display Port controller subnode to the MDSS node.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  arch/arm64/boot/dts/qcom/sm8550.dtsi | 89 
> +++-
>  1 file changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi 
> b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index 75cd374943eb..73524afc2e3a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -2495,6 +2495,13 @@ dpu_intf2_out: endpoint {
>   remote-endpoint = 
> <_dsi1_in>;
>   };
>   };
> +
> + port@2 {
> + reg = <2>;
> + dpu_intf0_out: endpoint {
> + remote-endpoint = 
> <_dp0_in>;
> + };
> + };
>   };
>  
>   mdp_opp_table: opp-table {
> @@ -2522,6 +2529,84 @@ opp-51400 {
>   };
>   };
>  
> + mdss_dp0: displayport-controller@ae9 {
> + compatible = "qcom,sm8550-dp", "qcom,sm8350-dp";
> + reg = <0 0xae9 0 0x200>,
> +   <0 0xae90200 0 0x200>,
> +   <0 0xae90400 0 0xc00>,
> +   <0 0xae91000 0 0x400>,
> +   <0 0xae91400 0 0x400>;
> + interrupt-parent = <>;
> + interrupts = <12>;
> + clocks = < DISP_CC_MDSS_AHB_CLK>,
> +  < DISP_CC_MDSS_DPTX0_AUX_CLK>,
> +  < DISP_CC_MDSS_DPTX0_LINK_CLK>,
> +  < 
> DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
> +  < 
> DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
> + clock-names = "core_iface",
> +   "core_aux",
> +   "ctrl_link",
> +   "ctrl_link_iface",
> +   "stream_pixel";
> +
> + assigned-clocks = < 
> DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
> +   < 
> DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
> + assigned-clock-parents = <_dp_qmpphy 
> QMP_USB43DP_DP_LINK_CLK>,
> +  <_dp_qmpphy 
> QMP_USB43DP_DP_VCO_DIV_CLK>;
> +
> + phys = <_dp_qmpphy QMP_USB43DP_DP_PHY>;
> + phy-names = "dp";
> +
> + #sound-dai-cells = <0>;
> +
> + operating-points-v2 = <_opp_table>;
> + power-domains = < SM8550_CX>;
MMCX

> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + mdss_dp0_in: endpoint {
> + remote-endpoint = 
> <_intf0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + mdss_dp0_out: endpoint {
> + };
> + };
> + };
> +
> + dp_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-16200 {
> + opp-hz = /bits/ 64 <16200>;
> + required-opps = 
> <_opp_low_svs>;
_d1

> + };
> +
> + opp-27000 {
> + opp-hz = /bits/ 64 <27000>;
> + required-opps = 
> <_opp_svs>;
low_svs

With these fixed:

Reviewed-by: Konrad Dybcio 

Konrad
> + };
> +
> + opp-54000 {
> +  

[Freedreno] [PATCH 2/2] arm64: dts: qcom: sm8550: add display port nodes

2023-06-01 Thread Neil Armstrong
Add the Display Port controller subnode to the MDSS node.

Signed-off-by: Neil Armstrong 
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 89 +++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi 
b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 75cd374943eb..73524afc2e3a 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2495,6 +2495,13 @@ dpu_intf2_out: endpoint {
remote-endpoint = 
<_dsi1_in>;
};
};
+
+   port@2 {
+   reg = <2>;
+   dpu_intf0_out: endpoint {
+   remote-endpoint = 
<_dp0_in>;
+   };
+   };
};
 
mdp_opp_table: opp-table {
@@ -2522,6 +2529,84 @@ opp-51400 {
};
};
 
+   mdss_dp0: displayport-controller@ae9 {
+   compatible = "qcom,sm8550-dp", "qcom,sm8350-dp";
+   reg = <0 0xae9 0 0x200>,
+ <0 0xae90200 0 0x200>,
+ <0 0xae90400 0 0xc00>,
+ <0 0xae91000 0 0x400>,
+ <0 0xae91400 0 0x400>;
+   interrupt-parent = <>;
+   interrupts = <12>;
+   clocks = < DISP_CC_MDSS_AHB_CLK>,
+< DISP_CC_MDSS_DPTX0_AUX_CLK>,
+< DISP_CC_MDSS_DPTX0_LINK_CLK>,
+< 
DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
+< 
DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
+   clock-names = "core_iface",
+ "core_aux",
+ "ctrl_link",
+ "ctrl_link_iface",
+ "stream_pixel";
+
+   assigned-clocks = < 
DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
+ < 
DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
+   assigned-clock-parents = <_dp_qmpphy 
QMP_USB43DP_DP_LINK_CLK>,
+<_dp_qmpphy 
QMP_USB43DP_DP_VCO_DIV_CLK>;
+
+   phys = <_dp_qmpphy QMP_USB43DP_DP_PHY>;
+   phy-names = "dp";
+
+   #sound-dai-cells = <0>;
+
+   operating-points-v2 = <_opp_table>;
+   power-domains = < SM8550_CX>;
+
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   mdss_dp0_in: endpoint {
+   remote-endpoint = 
<_intf0_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   mdss_dp0_out: endpoint {
+   };
+   };
+   };
+
+   dp_opp_table: opp-table {
+   compatible = "operating-points-v2";
+
+   opp-16200 {
+   opp-hz = /bits/ 64 <16200>;
+   required-opps = 
<_opp_low_svs>;
+   };
+
+   opp-27000 {
+   opp-hz = /bits/ 64 <27000>;
+   required-opps = 
<_opp_svs>;
+   };
+
+   opp-54000 {
+   opp-hz = /bits/ 64 <54000>;
+   required-opps = 
<_opp_svs_l1>;
+   };
+
+  

[Freedreno] [PATCH 1/2] dt-bindings: display: msm: dp-controller: document SM8550 compatible

2023-06-01 Thread Neil Armstrong
The SM8550 & SM8350 SoC shares the same DP TX IP version, use the
SM8350 compatible as fallback for SM8550.

Signed-off-by: Neil Armstrong 
---
 Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml 
b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index f0c2237d5f82..7a7cf3fb3e6d 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -29,6 +29,7 @@ properties:
   - items:
   - enum:
   - qcom,sm8450-dp
+  - qcom,sm8550-dp
   - const: qcom,sm8350-dp
 
   reg:

-- 
2.34.1



[Freedreno] [PATCH 0/2] arm64: dts: qcom: add DP Controller to SM8550 DTS

2023-06-01 Thread Neil Armstrong
The DP output is shared with the USB3 SuperSpeed lanes and is
usually connected to an USB-C port which Altmode is controlled
by the PMIC Glink infrastructure.

DT changes tying the DP controller to the USB-C port on the QRD
board will be sent later.

Signed-off-by: Neil Armstrong 
---
Neil Armstrong (2):
  dt-bindings: display: msm: dp-controller: document SM8550 compatible
  arm64: dts: qcom: sm8550: add display port nodes

 .../bindings/display/msm/dp-controller.yaml|  1 +
 arch/arm64/boot/dts/qcom/sm8550.dtsi   | 89 +-
 2 files changed, 88 insertions(+), 2 deletions(-)
---
base-commit: d4cee89031c80066ec461bb77b5e13a4f37d5fd2
change-id: 20230601-topic-sm8550-upstream-dp-b713ba275d7c

Best regards,
-- 
Neil Armstrong 



Re: [Freedreno] [PATCH v5 09/13] drm/tegra: Use regular fbdev I/O helpers

2023-06-01 Thread Thierry Reding
On Tue, May 30, 2023 at 05:12:24PM +0200, Thomas Zimmermann wrote:
> Use the regular fbdev helpers for framebuffer I/O instead of DRM's
> helpers. Tegra does not use damage handling, so DRM's fbdev helpers
> are mere wrappers around the fbdev code.
> 
> By using fbdev helpers directly within each DRM fbdev emulation,
> we can eventually remove DRM's wrapper functions entirely.
> 
> v4:
>   * use initializer macros for struct fb_ops
> v2:
>   * use FB_SYS_HELPERS option
> 
> Signed-off-by: Thomas Zimmermann 
> Acked-by: Sam Ravnborg 
> Cc: Thierry Reding 
> Cc: Mikko Perttunen 
> Cc: Jonathan Hunter 
> ---
>  drivers/gpu/drm/tegra/Kconfig | 1 +
>  drivers/gpu/drm/tegra/fbdev.c | 8 +++-
>  2 files changed, 4 insertions(+), 5 deletions(-)

Acked-by: Thierry Reding 


signature.asc
Description: PGP signature


Re: [Freedreno] [PATCH] drm/msm/a6xx: fix uninitialised lock in init error path

2023-06-01 Thread Johan Hovold
On Wed, May 31, 2023 at 07:22:49AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Wed, May 31, 2023 at 1:00 AM Johan Hovold  wrote:
> >
> > A recent commit started taking the GMU lock in the GPU destroy path,
> > which on GPU initialisation failure is called before the GMU and its
> > lock have been initialised.
> >
> > Make sure that the GMU has been initialised before taking the lock in
> > a6xx_destroy() and drop the now redundant check from a6xx_gmu_remove().
> >
> > Fixes: 4cd15a3e8b36 ("drm/msm/a6xx: Make GPU destroy a bit safer")
> > Cc: sta...@vger.kernel.org  # 6.3
> > Cc: Douglas Anderson 
> > Signed-off-by: Johan Hovold 
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 ++---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> I think Dmitry already posted a patch 1.5 months ago to fix this.
> 
> https://lore.kernel.org/r/20230410165908.3094626-1-dmitry.barysh...@linaro.org

Bah, I checked if Bjorn had hit this with his recent A690 v3 series and
posted a fix, but did not look further than that.

> Can you confirm that works for you?

That looks like it would work too, but I think I prefer my version which
keeps the initialisation of the GMU struct in a6xx_gmu_init().

Dmitry or Rob, could you see to that either version gets merged soon so
that we don't end up with even more people having to debug and fix the
same issue?

Johan