Re: [Freedreno] [PATCH 00/36] drm/amd/display: add AMD driver-specific properties for color mgmt

2023-05-29 Thread Dmitry Baryshkov

On 24/05/2023 01:14, Melissa Wen wrote:

This series is a refined version of our RFC [1] for AMD driver-specific
color management properties. It is a collection of contributions from
Joshua, Harry and I to enhance AMD KMS color pipeline for Steam
Deck/SteamOS by exposing the large set of color caps available in AMD
display HW.

Considering RFC feedback, this patchset differs from the previous one by
removing the KConfig option and just guarding driver-specific properties
with `AMD_PRIVATE_COLOR` - but we also removed the guards from internal
elements and operations. We stopped to advertise CRTC shaper and 3D LUTs
properties since they aren't in use in the Steam Deck color pipeline[2].
On the other hand, we keep mapping CRTC shaper and 3D LUTs (DM) to DC
MPC setup. We also improved curve calculations to take into account HW
color caps.

In short, for pre-blending, we added the following properties:
- plane degamma LUT and predefined transfer function;
- plane HDR multiplier
- plane shaper LUT/transfer function;
- plane 3D LUT; and finally,
- plane blend LUT/transfer function, just before blending.


This set of properties sounds interesting and not fully AMD-specific. 
Could you please consider moving them to the more generic location?


For the reference, MSM (Qualcomm) display hardware supports 
degamma/gamma LUTs for planes too. One of the suggested usecases for 
these degamma/gamma units is to support colorspace transfer functions.


Thus, at least some of these properties can be implemented in drm/msm 
driver too.



After blending, we already have DRM CRTC degamma/gamma LUTs and CTM,
therefore, we extend post-blending color pipeline with CRTC gamma
transfer function.

The first three patches are on DRM KMS side. We expose DRM property
helper for blob lookup and replacement so that we can use it for
managing driver-specific properties. We add a tracked for plane color
mgmt changes and increase the maximum number of properties to
accommodate this expansion.

The userspace case here is Gamescope which is the compositor for
SteamOS. It's already using all of this functionality to implement its
color management pipeline right now [3].

Current IGT tests kms_color and amdgpu/amd_color on DCN301 and DCN21 HW
preserve the same results with and without the guard.

Finally, I may have missed something, please let me know if that's the
case.

Best Regards,

Melissa Wen

[1] https://lore.kernel.org/dri-devel/20230423141051.702990-1-m...@igalia.com
[2] 
https://github.com/ValveSoftware/gamescope/blob/master/src/docs/Steam%20Deck%20Display%20Pipeline.png
[3] https://github.com/ValveSoftware/gamescope


Harry Wentland (2):
   drm/amd/display: fix segment distribution for linear LUTs
   drm/amd/display: fix the delta clamping for shaper LUT

Joshua Ashton (13):
   drm/amd/display: add plane degamma TF driver-specific property
   drm/amd/display: add plane HDR multiplier driver-specific property
   drm/amd/display: add plane blend LUT and TF driver-specific properties
   drm/amd/display: copy 3D LUT settings from crtc state to stream_update
   drm/amd/display: dynamically acquire 3DLUT resources for color changes
   drm/amd/display: add CRTC regamma TF support
   drm/amd/display: set sdr_ref_white_level to 80 for out_transfer_func
   drm/amd/display: add support for plane degamma TF and LUT properties
   drm/amd/display: add dc_fixpt_from_s3132 helper
   drm/adm/display: add HDR multiplier support
   drm/amd/display: handle empty LUTs in __set_input_tf
   drm/amd/display: add DRM plane blend LUT and TF support
   drm/amd/display: allow newer DC hardware to use degamma ROM for PQ/HLG

Melissa Wen (21):
   drm/drm_mode_object: increase max objects to accommodate new color
 props
   drm/drm_property: make replace_property_blob_from_id a DRM helper
   drm/drm_plane: track color mgmt changes per plane
   drm/amd/display: add CRTC driver-specific property for gamma TF
   drm/amd/display: add plane driver-specific properties for degamma LUT
   drm/amd/display: add plane 3D LUT driver-specific properties
   drm/amd/display: add plane shaper LUT driver-specific properties
   drm/amd/display: add plane shaper TF driver-private property
   drm/amd/display: add comments to describe DM crtc color mgmt behavior
   drm/amd/display: encapsulate atomic regamma operation
   drm/amd/display: update lut3d and shaper lut to stream
   drm/amd/display: allow BYPASS 3D LUT but keep shaper LUT settings
   drm/amd/display: handle MPC 3D LUT resources for a given context
   drm/amd/display: add CRTC 3D LUT support
   drm/amd/display: add CRTC shaper LUT support
   drm/amd/display: add CRTC shaper TF support
   drm/amd/display: mark plane as needing reset if plane color mgmt
 changes
   drm/amd/display: decouple steps for mapping CRTC degamma to DC plane
   drm/amd/display: reject atomic commit if setting both plane and CRTC
 degamma
   drm/amd/display: program DPP shaper and 3D LUT if updated
   drm/amd/display: add plane shaper/3D

Re: [Freedreno] [PATCH] drm/msm/dpu: use PINGPONG_NONE to unbind INTF from PP

2023-05-29 Thread Dmitry Baryshkov
On Tue, 30 May 2023 at 00:46, Marijn Suijten
 wrote:
>
> On 2023-05-26 12:09:45, Dmitry Baryshkov wrote:
> > Currently the driver passes the PINGPONG index to
> > dpu_hw_intf_ops::bind_pingpong_blk() callback and uses separate boolean
> > flag to tell whether INTF should be bound or unbound. Simplify this by
> > passing PINGPONG_NONE in case of unbinding and drop the flag completely.
>
> Perhaps worth mentioning that this flag was only recently introduced
> (and link to it as a dependency under the cut),

The patch is already a part of linux-next. This is the usual boundary
of skipping the dependencies.

>  as well as explain that
> the passed `enum dpu_pingpong` value is bogus when enable=false because
> it is not part of the value written to the register for the
> "unbind/disable" case?

Good suggestion.

>  See for example the wording I suggested on the
> patch that introduces and uses PINGPONG_NONE.
>
> > Signed-off-by: Dmitry Baryshkov 
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c  | 4 ++--
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 1 -
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c  | 3 +--
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h  | 1 -
>
> How about appending/sending another patch that drops this from
> dpu_hw_wb.c as well?

Ack, nice catch.

>
> >  5 files changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index ebe34eda6e50..7fd3a13ac226 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -2102,8 +2102,8 @@ void dpu_encoder_helper_phys_cleanup(struct 
> > dpu_encoder_phys *phys_enc)
> >   for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> >   if (dpu_enc->phys_encs[i] && 
> > phys_enc->hw_intf->ops.bind_pingpong_blk)
> >   phys_enc->hw_intf->ops.bind_pingpong_blk(
> > - 
> > dpu_enc->phys_encs[i]->hw_intf, false,
> > - 
> > dpu_enc->phys_encs[i]->hw_pp->idx);
> > + 
> > dpu_enc->phys_encs[i]->hw_intf,
> > + PINGPONG_NONE);
> >
> >   /* mark INTF flush as pending */
> >   if (phys_enc->hw_ctl->ops.update_pending_flush_intf)
> > 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 1a4c20f02312..3130c86a32cc 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
> > @@ -66,7 +66,6 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
> >   if (test_bit(DPU_CTL_ACTIVE_CFG, &ctl->caps->features) && 
> > phys_enc->hw_intf->ops.bind_pingpong_blk)
> >   phys_enc->hw_intf->ops.bind_pingpong_blk(
> >   phys_enc->hw_intf,
> > - true,
> >   phys_enc->hw_pp->idx);
> >
> >   if (phys_enc->hw_intf->ops.enable_compression)
> > @@ -556,8 +555,7 @@ static void dpu_encoder_phys_cmd_disable(struct 
> > dpu_encoder_phys *phys_enc)
> >   if (phys_enc->hw_intf->ops.bind_pingpong_blk) {
> >   phys_enc->hw_intf->ops.bind_pingpong_blk(
> >   phys_enc->hw_intf,
> > - false,
> > - phys_enc->hw_pp->idx);
> > + PINGPONG_NONE);
>
> Is there also a cleanup state where hw_pp is assigned back to NULL?

No. None of the encoder resources are reassigned to NULL. I will tend
this topic during vN of resource allocation rework.

>
> >   ctl = phys_enc->hw_ctl;
> >   ctl->ops.update_pending_flush_intf(ctl, phys_enc->intf_idx);
> > 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..220020273304 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
> > @@ -287,7 +287,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
> >   if (phys_enc->hw_intf->ops.bind_pingpong_blk)
> >   phys_enc->hw_intf->ops.bind_pingpong_blk(
> >   phys_enc->hw_intf,
> > - true,
> >   phys_enc->hw_pp->idx);
> >
> >   if (phys_enc->hw_pp->merge_3d)
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > index a2486f99d3c2..918d154848b9 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_

Re: [Freedreno] [PATCH] drm/msm/dpu: use PINGPONG_NONE to unbind INTF from PP

2023-05-29 Thread Marijn Suijten
On 2023-05-26 12:09:45, Dmitry Baryshkov wrote:
> Currently the driver passes the PINGPONG index to
> dpu_hw_intf_ops::bind_pingpong_blk() callback and uses separate boolean
> flag to tell whether INTF should be bound or unbound. Simplify this by
> passing PINGPONG_NONE in case of unbinding and drop the flag completely.

Perhaps worth mentioning that this flag was only recently introduced
(and link to it as a dependency under the cut), as well as explain that
the passed `enum dpu_pingpong` value is bogus when enable=false because
it is not part of the value written to the register for the
"unbind/disable" case?  See for example the wording I suggested on the
patch that introduces and uses PINGPONG_NONE.

> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c  | 4 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 1 -
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c  | 3 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h  | 1 -

How about appending/sending another patch that drops this from
dpu_hw_wb.c as well?

>  5 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ebe34eda6e50..7fd3a13ac226 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2102,8 +2102,8 @@ void dpu_encoder_helper_phys_cleanup(struct 
> dpu_encoder_phys *phys_enc)
>   for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>   if (dpu_enc->phys_encs[i] && 
> phys_enc->hw_intf->ops.bind_pingpong_blk)
>   phys_enc->hw_intf->ops.bind_pingpong_blk(
> - dpu_enc->phys_encs[i]->hw_intf, 
> false,
> - 
> dpu_enc->phys_encs[i]->hw_pp->idx);
> + dpu_enc->phys_encs[i]->hw_intf,
> + PINGPONG_NONE);
>  
>   /* mark INTF flush as pending */
>   if (phys_enc->hw_ctl->ops.update_pending_flush_intf)
> 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 1a4c20f02312..3130c86a32cc 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
> @@ -66,7 +66,6 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
>   if (test_bit(DPU_CTL_ACTIVE_CFG, &ctl->caps->features) && 
> phys_enc->hw_intf->ops.bind_pingpong_blk)
>   phys_enc->hw_intf->ops.bind_pingpong_blk(
>   phys_enc->hw_intf,
> - true,
>   phys_enc->hw_pp->idx);
>  
>   if (phys_enc->hw_intf->ops.enable_compression)
> @@ -556,8 +555,7 @@ static void dpu_encoder_phys_cmd_disable(struct 
> dpu_encoder_phys *phys_enc)
>   if (phys_enc->hw_intf->ops.bind_pingpong_blk) {
>   phys_enc->hw_intf->ops.bind_pingpong_blk(
>   phys_enc->hw_intf,
> - false,
> - phys_enc->hw_pp->idx);
> + PINGPONG_NONE);

Is there also a cleanup state where hw_pp is assigned back to NULL?

>   ctl = phys_enc->hw_ctl;
>   ctl->ops.update_pending_flush_intf(ctl, phys_enc->intf_idx);
> 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..220020273304 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
> @@ -287,7 +287,6 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
>   if (phys_enc->hw_intf->ops.bind_pingpong_blk)
>   phys_enc->hw_intf->ops.bind_pingpong_blk(
>   phys_enc->hw_intf,
> - true,
>   phys_enc->hw_pp->idx);
>  
>   if (phys_enc->hw_pp->merge_3d)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index a2486f99d3c2..918d154848b9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -268,7 +268,6 @@ static void dpu_hw_intf_setup_prg_fetch(
>  
>  static void dpu_hw_intf_bind_pingpong_blk(
>   struct dpu_hw_intf *intf,
> - bool enable,
>   const enum dpu_pingpong pp)
>  {
>   struct dpu_hw_blk_reg_map *c = &intf->hw;
> @@ -277,7 +276,7 @@ static void dpu_hw_intf_bind_pingpong_blk(
>   mux_cfg = DPU_REG_READ(c, INTF_MUX);
>   mux_cfg &= ~0xf;
>  
> - if (enable)
> + if (pp != PINGPONG_NONE)

Kuogee just use

Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

2023-05-29 Thread Marijn Suijten
On 2023-05-24 12:18:09, Abhinav Kumar wrote:
> 
> 
> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> > On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >>
> >>
> >> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>> Drop SSPP-specifig debugfs register dumps in favour of using
> >>> debugfs/dri/0/kms or devcoredump.
> >>>
> >>
> >> I did see another series which removes src_blk from the catalog (I am
> >> yet to review that one) . Lets assume that one is fine and this change
> >> will be going on top of that one right?
> > 
> > It replaces src_blk with directly accessing the blk (non-sub-block)
> > directly, because they were overlapping anyway.
> > 
> >> The concern I have with this change is that although I do agree that we
> >> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> >> times and it works pretty well ), devcoredump does not have the support
> >> to dump sub-blocks . Something which we should add with priority because
> >> even with DSC blocks with the separation of enc/ctl blocks we need that
> >> like I wrote in one of the responses.
> >>
> >> So the "len" of the blocks having sub-blocks will be ignored in favor of
> >> the len of the sub-blocks.
> > 
> > The sub-blocks are not always contiguous with their parent block, are
> > they?  It's probably better to print the sub-blocks separately with
> 
> Yes, not contiguous otherwise we could have just had them in one big range.
> 
> > clear headers anyway rather than dumping the range parent_blk_base to
> > max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> > 
> > - Marijn
> 
> When I meant sub-block support to devcoredump, this is how I visualize 
> them to be printed
> 
> =SSPP xxx ===
> =SSPP_CSC ===(for SSPP_xxx)
> =SSPP_QSEED =(for SSPP_xxx)

Yeah something along those lines, though I don't really like the `(for
SSPP_xxx)` suffix which we should either drop entirely and make the
"heading" less of a "heading"

= SSPP xxx ===
...
- SSPP_CSC ---
...
- SSPP_QSEED -
...

And/or inline the numbers:

= SSPP xxx ===
...
--- SSPP_xxx_CSC -
...
-- SSPP_xxx_QSEED 
...

Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
that clearly tells the blocks and sub-blocks apart.

- Marijn


Re: [Freedreno] [PATCH v4 13/13] drm/i915: Implement dedicated fbdev I/O helpers

2023-05-29 Thread Sam Ravnborg
Hi Thomas,

On Wed, May 24, 2023 at 11:21:50AM +0200, Thomas Zimmermann wrote:
> Implement dedicated fbdev helpers for framebuffer I/O instead
> of using DRM's helpers. Use an fbdev generator macro for
> deferred I/O to create the fbdev callbacks. i915 was the only
> caller of the DRM helpers, so remove them from the helper module.
> 
> i915's fbdev emulation is still incomplete as it doesn't implement
> deferred I/O and damage handling for mmaped pages.
> 
> v4:
>   * generate deferred-I/O helpers
>   * use initializer macros for fb_ops
> v2:
>   * use FB_IO_HELPERS options
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Tvrtko Ursulin 
> Cc: "Ville Syrjälä" 
> ---
>  drivers/gpu/drm/Kconfig|   3 -
>  drivers/gpu/drm/drm_fb_helper.c| 107 -
>  drivers/gpu/drm/i915/Kconfig   |   1 +
>  drivers/gpu/drm/i915/display/intel_fbdev.c |  14 +--
>  include/drm/drm_fb_helper.h|  39 
>  5 files changed, 9 insertions(+), 155 deletions(-)

Nice diffstat!
Assuming there is a good explanation on the dirty check:
Reviewed-by: Sam Ravnborg 


Re: [Freedreno] [PATCH v4 11/13] drm/fb-helper: Export helpers for marking damage areas

2023-05-29 Thread Sam Ravnborg
On Wed, May 24, 2023 at 11:21:48AM +0200, Thomas Zimmermann wrote:
> Export drm_fb_helper_damage() and drm_fb_helper_damage_range(), which
> handle damage areas for fbdev emulation. This is a temporary export
> that allows to move the DRM I/O helpers for fbdev into drivers. Only
> fbdev-generic and i915 need them. Both will be updated to implement
> damage handling by themselves and the exported functions will be removed.
> 
> v4:
>   * update interfaces
> 
> Signed-off-by: Thomas Zimmermann 

Assuming there is a good answer why there is no dirty check:
Reviewed-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 22 ++
>  include/drm/drm_fb_helper.h |  3 +++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f0e9549b6bd7..cb03099fd2e3 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -670,6 +670,28 @@ static void drm_fb_helper_memory_range_to_clip(struct 
> fb_info *info, off_t off,
>   drm_rect_init(clip, x1, y1, x2 - x1, y2 - y1);
>  }
>  
> +/* Don't use in new code. */
> +void drm_fb_helper_damage_range(struct fb_info *info, off_t off, size_t len)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct drm_rect damage_area;
> +
> + drm_fb_helper_memory_range_to_clip(info, off, len, &damage_area);
> + drm_fb_helper_damage(fb_helper, damage_area.x1, damage_area.y1,
> +  drm_rect_width(&damage_area),
> +  drm_rect_height(&damage_area));
> +}
> +EXPORT_SYMBOL(drm_fb_helper_damage_range);
> +
> +/* Don't use in new code. */
> +void drm_fb_helper_damage_area(struct fb_info *info, u32 x, u32 y, u32 
> width, u32 height)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> +
> + drm_fb_helper_damage(fb_helper, x, y, width, height);
> +}
> +EXPORT_SYMBOL(drm_fb_helper_damage_area);
> +
>  /**
>   * drm_fb_helper_deferred_io() - fbdev deferred_io callback function
>   * @info: fb_info struct pointer
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 72032c354a30..7d5804882be7 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -253,6 +253,9 @@ void drm_fb_helper_fill_info(struct fb_info *info,
>struct drm_fb_helper *fb_helper,
>struct drm_fb_helper_surface_size *sizes);
>  
> +void drm_fb_helper_damage_range(struct fb_info *info, off_t off, size_t len);
> +void drm_fb_helper_damage_area(struct fb_info *info, u32 x, u32 y, u32 
> width, u32 height);
> +
>  void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head 
> *pagereflist);
>  
>  ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
> -- 
> 2.40.1


Re: [Freedreno] [PATCH v4 00/13] drm/fbdev: Remove DRM's helpers for fbdev I/O

2023-05-29 Thread Sam Ravnborg
Hi Thomas.

> v4:
>   * use initializer and generator macros for struct fb_ops
>   * partially support damage handling in msm (Dmitri)

I like the macros. They make it simpler and we do not spread the _cfb_
misname to more files.


> v3:
>   * fix Kconfig options (Jingfeng)
>   * minimize changes to exynos (Sam)
> v2:
>   * simplify Kconfig handling (Sam)
> 
> Thomas Zimmermann (13):
>   fbdev: Add Kconfig options to select different fb_ops helpers
>   fbdev: Add initializer macros for struct fb_ops


>   drm/armada: Use regular fbdev I/O helpers
>   drm/exynos: Use regular fbdev I/O helpers
>   drm/gma500: Use regular fbdev I/O helpers
>   drm/radeon: Use regular fbdev I/O helpers
>   drm/fbdev-dma: Use regular fbdev I/O helpers
>   drm/msm: Use regular fbdev I/O helpers
>   drm/omapdrm: Use regular fbdev I/O helpers
>   drm/tegra: Use regular fbdev I/O helpers
These are all:
Acked-by: Sam Ravnborg 


Re: [Freedreno] [PATCH v4 13/13] drm/i915: Implement dedicated fbdev I/O helpers

2023-05-29 Thread Sam Ravnborg
Hi Thomas,

On Wed, May 24, 2023 at 11:21:50AM +0200, Thomas Zimmermann wrote:
> Implement dedicated fbdev helpers for framebuffer I/O instead
> of using DRM's helpers. Use an fbdev generator macro for
> deferred I/O to create the fbdev callbacks. i915 was the only
> caller of the DRM helpers, so remove them from the helper module.
> 
> i915's fbdev emulation is still incomplete as it doesn't implement
> deferred I/O and damage handling for mmaped pages.
> 
> v4:
>   * generate deferred-I/O helpers
>   * use initializer macros for fb_ops
> v2:
>   * use FB_IO_HELPERS options
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Tvrtko Ursulin 
> Cc: "Ville Syrjälä" 
> ---
>  drivers/gpu/drm/Kconfig|   3 -
>  drivers/gpu/drm/drm_fb_helper.c| 107 -
>  drivers/gpu/drm/i915/Kconfig   |   1 +
>  drivers/gpu/drm/i915/display/intel_fbdev.c |  14 +--
>  include/drm/drm_fb_helper.h|  39 
>  5 files changed, 9 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 92a782827b7b..bb2e48cc6cd6 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -133,9 +133,6 @@ config DRM_FBDEV_EMULATION
>   bool "Enable legacy fbdev support for your modesetting driver"
>   depends on DRM_KMS_HELPER
>   depends on FB=y || FB=DRM_KMS_HELPER
> - select FB_CFB_FILLRECT
> - select FB_CFB_COPYAREA
> - select FB_CFB_IMAGEBLIT
>   select FRAMEBUFFER_CONSOLE if !EXPERT
>   select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE
>   default y
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bab6b252f02a..9978147bbc8a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -736,113 +736,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, 
> struct list_head *pagerefli
>  }
>  EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>  
> -/**
> - * drm_fb_helper_cfb_read - Implements struct &fb_ops.fb_read for I/O memory
> - * @info: fb_info struct pointer
> - * @buf: userspace buffer to read from framebuffer memory
> - * @count: number of bytes to read from framebuffer memory
> - * @ppos: read offset within framebuffer memory
> - *
> - * Returns:
> - * The number of bytes read on success, or an error code otherwise.
> - */
> -ssize_t drm_fb_helper_cfb_read(struct fb_info *info, char __user *buf,
> -size_t count, loff_t *ppos)
> -{
> - return fb_io_read(info, buf, count, ppos);
> -}
> -EXPORT_SYMBOL(drm_fb_helper_cfb_read);
> -
> -/**
> - * drm_fb_helper_cfb_write - Implements struct &fb_ops.fb_write for I/O 
> memory
> - * @info: fb_info struct pointer
> - * @buf: userspace buffer to write to framebuffer memory
> - * @count: number of bytes to write to framebuffer memory
> - * @ppos: write offset within framebuffer memory
> - *
> - * Returns:
> - * The number of bytes written on success, or an error code otherwise.
> - */
> -ssize_t drm_fb_helper_cfb_write(struct fb_info *info, const char __user *buf,
> - size_t count, loff_t *ppos)
> -{
> - struct drm_fb_helper *helper = info->par;
> - loff_t pos = *ppos;
> - ssize_t ret;
> - struct drm_rect damage_area;
> -
> - ret = fb_io_write(info, buf, count, ppos);
> - if (ret <= 0)
> - return ret;
> -
> - if (helper->funcs->fb_dirty) {
> - drm_fb_helper_memory_range_to_clip(info, pos, ret, 
> &damage_area);
> - drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1,
> -  drm_rect_width(&damage_area),
> -  drm_rect_height(&damage_area));
> - }

The generated helpers do not have the if (helper->funcs->fb_dirty)
check.
Is this implemented somewhere else that I missed?

Sam


> -
> - return ret;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_cfb_write);
> -
> -/**
> - * drm_fb_helper_cfb_fillrect - wrapper around cfb_fillrect
> - * @info: fbdev registered by the helper
> - * @rect: info about rectangle to fill
> - *
> - * A wrapper around cfb_fillrect implemented by fbdev core
> - */
> -void drm_fb_helper_cfb_fillrect(struct fb_info *info,
> - const struct fb_fillrect *rect)
> -{
> - struct drm_fb_helper *helper = info->par;
> -
> - cfb_fillrect(info, rect);
> -
> - if (helper->funcs->fb_dirty)
> - drm_fb_helper_damage(helper, rect->dx, rect->dy, rect->width, 
> rect->height);
> -}
> -EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect);
> -
> -/**
> - * drm_fb_helper_cfb_copyarea - wrapper around cfb_copyarea
> - * @info: fbdev registered by the helper
> - * @area: info about area to copy
> - *
> - * A wrapper around cfb_copyarea implemented by fbdev core
> - */
> -void drm_fb_helper_cfb_copyarea(struct fb_info *info,
> -   

Re: [Freedreno] [PATCH v4 03/13] drm/armada: Use regular fbdev I/O helpers

2023-05-29 Thread Sam Ravnborg
On Wed, May 24, 2023 at 11:21:40AM +0200, Thomas Zimmermann wrote:
> Use the regular fbdev helpers for framebuffer I/O instead of DRM's
> helpers. Armada 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_IO_HELPERS option
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Russell King 
Acked-by: Sam Ravnborg 


Re: [Freedreno] [PATCH v4 02/13] fbdev: Add initializer macros for struct fb_ops

2023-05-29 Thread Sam Ravnborg
On Wed, May 24, 2023 at 11:21:39AM +0200, Thomas Zimmermann wrote:
> For framebuffers in I/O and system memory, add macros that set
> struct fb_ops to the respective callback functions.
> 
> For deferred I/O, add macros that generate callback functions with
> damage handling. Add initializer macros that set struct fb_ops to
> the generated callbacks.
> 
> These macros can remove a lot boilerplate code from fbdev drivers.
> The drivers are supposed to use the macro that is required for its
> framebuffer. Each macro is split into smaller helpers, so that
> drivers with non-standard callbacks can pick and customize callbacks
> as needed. There are individual helper macros for read/write, mmap
> and drawing.
> 
> Signed-off-by: Thomas Zimmermann 
I am not a fan of public functions/macros names __something.
But I see it used in so many places, so maybe it is just me.
And everything looks consistent, so OK.

With the white space issues fixed:
Reviewed-by: Sam Ravnborg 


Re: [Freedreno] [PATCH v4 01/13] fbdev: Add Kconfig options to select different fb_ops helpers

2023-05-29 Thread Sam Ravnborg
Hi Thomas,

On Wed, May 24, 2023 at 11:21:38AM +0200, Thomas Zimmermann wrote:
> Many fbdev drivers use the same set of fb_ops helpers. Add Kconfig
> options to select them at once. This will help with making DRM's
> fbdev emulation code more modular, but can also be used to simplify
> fbdev's driver configs.
> 
> v3:
>   * fix select statement (Jingfeng)
> 
> Signed-off-by: Thomas Zimmermann 
I like these, thanks.
Reviewed-by: Sam Ravnborg 


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

2023-05-29 Thread Conor Dooley
On Mon, May 29, 2023 at 11:43:58AM +0200, Luca Weiss wrote:
> The MSM8226 SoC uses a slightly different 28nm dsi phy. Add a new
> compatible for it.
> 
> Signed-off-by: Luca Weiss 
> ---
>  Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml | 1 +
>  Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml| 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml 
> b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
> index cf4a338c4661..bd70c3873ca9 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
> @@ -18,6 +18,7 @@ properties:
>- qcom,dsi-phy-28nm-hpm
>- qcom,dsi-phy-28nm-hpm-fam-b
>- qcom,dsi-phy-28nm-lp
> +  - qcom,dsi-phy-28nm-8226

How come the order is different in both places?

Acked-by: Conor Dooley 

Thanks,
Conor.

>- qcom,dsi-phy-28nm-8960
>  
>reg:
> 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
> 


signature.asc
Description: PGP signature


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

2023-05-29 Thread Conor Dooley
On Mon, May 29, 2023 at 11:44:00AM +0200, Luca Weiss wrote:
> Add the compatible for the MDP5 found on MSM8226.
> 
> Signed-off-by: Luca Weiss 

Acked-by: Conor Dooley 

Thanks,
Conor.


signature.asc
Description: PGP signature


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

2023-05-29 Thread Conor Dooley
On Mon, May 29, 2023 at 11:43:59AM +0200, Luca Weiss wrote:
> Add the compatible for the DSI found on MSM8226.
> 
> Signed-off-by: Luca Weiss 

Acked-by: Conor Dooley 

Thanks,
Conor.


signature.asc
Description: PGP signature


[Freedreno] [PATCH v8 15/18] drm/msm/a6xx: Use "else if" in GPU speedbin rev matching

2023-05-29 Thread Konrad Dybcio
The GPU can only be one at a time. Turn a series of ifs into if +
elseifs to save some CPU cycles.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 1a29e7dd9975..5faa85543428 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2170,16 +2170,16 @@ static u32 fuse_to_supp_hw(struct device *dev, struct 
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
 
-   if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
+   else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
val = a619_get_speed_bin(fuse);
 
-   if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
+   else if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
val = adreno_7c3_get_speed_bin(fuse);
 
-   if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
+   else if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
val = a640_get_speed_bin(fuse);
 
-   if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
+   else if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
val = a650_get_speed_bin(fuse);
 
if (val == UINT_MAX) {

-- 
2.40.1



[Freedreno] [PATCH v8 17/18] drm/msm/a6xx: Add A619_holi speedbin support

2023-05-29 Thread Konrad Dybcio
A619_holi is implemented on at least two SoCs: SM4350 (holi) and SM6375
(blair). This is what seems to be a first occurrence of this happening,
but it's easy to overcome by guarding the SoC-specific fuse values with
of_machine_is_compatible(). Do just that to enable frequency limiting
on these SoCs.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ca4ffa44097e..d046af5f6de2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2110,6 +2110,34 @@ static u32 a618_get_speed_bin(u32 fuse)
return UINT_MAX;
 }
 
+static u32 a619_holi_get_speed_bin(u32 fuse)
+{
+   /*
+* There are (at least) two SoCs implementing A619_holi: SM4350 (holi)
+* and SM6375 (blair). Limit the fuse matching to the corresponding
+* SoC to prevent bogus frequency setting (as improbable as it may be,
+* given unexpected fuse values are.. unexpected! But still possible.)
+*/
+
+   if (fuse == 0)
+   return 0;
+
+   if (of_machine_is_compatible("qcom,sm4350")) {
+   if (fuse == 138)
+   return 1;
+   else if (fuse == 92)
+   return 2;
+   } else if (of_machine_is_compatible("qcom,sm6375")) {
+   if (fuse == 190)
+   return 1;
+   else if (fuse == 177)
+   return 2;
+   } else
+   pr_warn("Unknown SoC implementing A619_holi!\n");
+
+   return UINT_MAX;
+}
+
 static u32 a619_get_speed_bin(u32 fuse)
 {
if (fuse == 0)
@@ -2170,6 +2198,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct 
adreno_gpu *adreno_gpu, u3
if (adreno_is_a618(adreno_gpu))
val = a618_get_speed_bin(fuse);
 
+   else if (adreno_is_a619_holi(adreno_gpu))
+   val = a619_holi_get_speed_bin(fuse);
+
else if (adreno_is_a619(adreno_gpu))
val = a619_get_speed_bin(fuse);
 

-- 
2.40.1



[Freedreno] [PATCH v8 13/18] drm/msm/a6xx: Add A610 support

2023-05-29 Thread Konrad Dybcio
A610 is one of (if not the) lowest-tier SKUs in the A6XX family. It
features no GMU, as it's implemented solely on SoCs with SMD_RPM.
What's more interesting is that it does not feature a VDDGX line
either, being powered solely by VDDCX and has an unfortunate hardware
quirk that makes its reset line broken - after a couple of assert/
deassert cycles, it will hang for good and will not wake up again.

This GPU requires mesa changes for proper rendering, and lots of them
at that. The command streams are quite far away from any other A6XX
GPU and hence it needs special care. This patch was validated both
by running an (incomplete) downstream mesa with some hacks (frames
rendered correctly, though some instructions made the GPU hangcheck
which is expected - garbage in, garbage out) and by replaying RD
traces captured with the downstream KGSL driver - no crashes there,
ever.

Add support for this GPU on the kernel side, which comes down to
pretty simply adding A612 HWCG tables, altering a few values and
adding a special case for handling the reset line.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c  | 101 +
 drivers/gpu/drm/msm/adreno/adreno_device.c |  12 
 drivers/gpu/drm/msm/adreno/adreno_gpu.h|   8 ++-
 3 files changed, 108 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index bb04f65e6f68..c0d5973320d9 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -252,6 +252,56 @@ static void a6xx_submit(struct msm_gpu *gpu, struct 
msm_gem_submit *submit)
a6xx_flush(gpu, ring);
 }
 
+const struct adreno_reglist a612_hwcg[] = {
+   {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220},
+   {REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0081},
+   {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xf3cf},
+   {REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002},
+   {REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001},
+   {REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007},
+   {REG_A6XX_RBBM_CLOCK_CNTL_RB0, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL2_RB0, 0x0120},
+   {REG_A6XX_RBBM_CLOCK_CNTL_CCU0, 0x2220},
+   {REG_A6XX_RBBM_CLOCK_HYST_RB_CCU0, 0x00040f00},
+   {REG_A6XX_RBBM_CLOCK_CNTL_RAC, 0x05522022},
+   {REG_A6XX_RBBM_CLOCK_CNTL2_RAC, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY_RAC, 0x0011},
+   {REG_A6XX_RBBM_CLOCK_HYST_RAC, 0x00445044},
+   {REG_A6XX_RBBM_CLOCK_CNTL_TSE_RAS_RBBM, 0x0422},
+   {REG_A6XX_RBBM_CLOCK_MODE_VFD, 0x},
+   {REG_A6XX_RBBM_CLOCK_MODE_GPC, 0x0222},
+   {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ_2, 0x0002},
+   {REG_A6XX_RBBM_CLOCK_MODE_HLSQ, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY_TSE_RAS_RBBM, 0x4000},
+   {REG_A6XX_RBBM_CLOCK_DELAY_VFD, 0x},
+   {REG_A6XX_RBBM_CLOCK_DELAY_GPC, 0x0200},
+   {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST_TSE_RAS_RBBM, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST_VFD, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST_GPC, 0x04104004},
+   {REG_A6XX_RBBM_CLOCK_HYST_HLSQ, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL_UCHE, 0x},
+   {REG_A6XX_RBBM_CLOCK_HYST_UCHE, 0x0004},
+   {REG_A6XX_RBBM_CLOCK_DELAY_UCHE, 0x0002},
+   {REG_A6XX_RBBM_ISDB_CNT, 0x0182},
+   {REG_A6XX_RBBM_RAC_THRESHOLD_CNT, 0x},
+   {REG_A6XX_RBBM_SP_HYST_CNT, 0x},
+   {REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x0222},
+   {REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x0111},
+   {REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x0555},
+   {},
+};
+
 /* For a615 family (a615, a616, a618 and a619) */
 const struct adreno_reglist a615_hwcg[] = {
{REG_A6XX_RBBM_CLOCK_CNTL_SP0,  0x0222},
@@ -602,6 +652,8 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
 
if (adreno_is_a630(adreno_gpu))
clock_cntl_on = 0x8aa8aa02;
+   else if (adreno_is_a610(adreno_gpu))
+   clock_cntl_on = 0xaaa8aa82;
else
clock_cntl_on = 0x8aa8aa82;
 
@@ -612,13 +664,15 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
return;
 
/* Disable SP clock before programming HWCG registers */
-   gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 1, 0);
+   if (!adreno_is_a610(adreno_gpu))
+

[Freedreno] [PATCH v8 16/18] drm/msm/a6xx: Use adreno_is_aXYZ macros in speedbin matching

2023-05-29 Thread Konrad Dybcio
Before transitioning to using per-SoC and not per-Adreno speedbin
fuse values (need another patchset to land elsewhere), a good
improvement/stopgap solution is to use adreno_is_aXYZ macros in
place of explicit revision matching. Do so to allow differentiating
between A619 and A619_holi.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 18 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 14 --
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 5faa85543428..ca4ffa44097e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2163,23 +2163,23 @@ static u32 adreno_7c3_get_speed_bin(u32 fuse)
return UINT_MAX;
 }
 
-static u32 fuse_to_supp_hw(struct device *dev, struct adreno_rev rev, u32 fuse)
+static u32 fuse_to_supp_hw(struct device *dev, struct adreno_gpu *adreno_gpu, 
u32 fuse)
 {
u32 val = UINT_MAX;
 
-   if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
+   if (adreno_is_a618(adreno_gpu))
val = a618_get_speed_bin(fuse);
 
-   else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
+   else if (adreno_is_a619(adreno_gpu))
val = a619_get_speed_bin(fuse);
 
-   else if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
+   else if (adreno_is_7c3(adreno_gpu))
val = adreno_7c3_get_speed_bin(fuse);
 
-   else if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
+   else if (adreno_is_a640(adreno_gpu))
val = a640_get_speed_bin(fuse);
 
-   else if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
+   else if (adreno_is_a650(adreno_gpu))
val = a650_get_speed_bin(fuse);
 
if (val == UINT_MAX) {
@@ -2192,7 +2192,7 @@ static u32 fuse_to_supp_hw(struct device *dev, struct 
adreno_rev rev, u32 fuse)
return (1 << val);
 }
 
-static int a6xx_set_supported_hw(struct device *dev, struct adreno_rev rev)
+static int a6xx_set_supported_hw(struct device *dev, struct adreno_gpu 
*adreno_gpu)
 {
u32 supp_hw;
u32 speedbin;
@@ -2211,7 +2211,7 @@ static int a6xx_set_supported_hw(struct device *dev, 
struct adreno_rev rev)
return ret;
}
 
-   supp_hw = fuse_to_supp_hw(dev, rev, speedbin);
+   supp_hw = fuse_to_supp_hw(dev, adreno_gpu, speedbin);
 
ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
if (ret)
@@ -2330,7 +2330,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 
a6xx_llc_slices_init(pdev, a6xx_gpu);
 
-   ret = a6xx_set_supported_hw(&pdev->dev, config->rev);
+   ret = a6xx_set_supported_hw(&pdev->dev, adreno_gpu);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 7a5d595d4b99..21513cec038f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -268,9 +268,9 @@ static inline int adreno_is_a630(struct adreno_gpu *gpu)
return gpu->revn == 630;
 }
 
-static inline int adreno_is_a640_family(struct adreno_gpu *gpu)
+static inline int adreno_is_a640(struct adreno_gpu *gpu)
 {
-   return (gpu->revn == 640) || (gpu->revn == 680);
+   return gpu->revn == 640;
 }
 
 static inline int adreno_is_a650(struct adreno_gpu *gpu)
@@ -289,6 +289,11 @@ static inline int adreno_is_a660(struct adreno_gpu *gpu)
return gpu->revn == 660;
 }
 
+static inline int adreno_is_a680(struct adreno_gpu *gpu)
+{
+   return gpu->revn == 680;
+}
+
 /* check for a615, a616, a618, a619 or any derivatives */
 static inline int adreno_is_a615_family(struct adreno_gpu *gpu)
 {
@@ -306,6 +311,11 @@ static inline int adreno_is_a650_family(struct adreno_gpu 
*gpu)
return gpu->revn == 650 || gpu->revn == 620 || 
adreno_is_a660_family(gpu);
 }
 
+static inline int adreno_is_a640_family(struct adreno_gpu *gpu)
+{
+   return adreno_is_a640(gpu) || adreno_is_a680(gpu);
+}
+
 u64 adreno_private_address_space_size(struct msm_gpu *gpu);
 int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
 uint32_t param, uint64_t *value, uint32_t *len);

-- 
2.40.1



[Freedreno] [PATCH v8 18/18] drm/msm/a6xx: Add A610 speedbin support

2023-05-29 Thread Konrad Dybcio
A610 is implemented on at least three SoCs: SM6115 (bengal), SM6125
(trinket) and SM6225 (khaje). Trinket does not support speed binning
(only a single SKU exists) and we don't yet support khaje upstream.
Hence, add a fuse mapping table for bengal to allow for per-chip
frequency limiting.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index d046af5f6de2..c304fa118cff 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2098,6 +2098,30 @@ static bool a6xx_progress(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring)
return progress;
 }
 
+static u32 a610_get_speed_bin(u32 fuse)
+{
+   /*
+* There are (at least) three SoCs implementing A610: SM6125 (trinket),
+* SM6115 (bengal) and SM6225 (khaje). Trinket does not have 
speedbinning,
+* as only a single SKU exists and we don't support khaje upstream yet.
+* Hence, this matching table is only valid for bengal and can be easily
+* expanded if need be.
+*/
+
+   if (fuse == 0)
+   return 0;
+   else if (fuse == 206)
+   return 1;
+   else if (fuse == 200)
+   return 2;
+   else if (fuse == 157)
+   return 3;
+   else if (fuse == 127)
+   return 4;
+
+   return UINT_MAX;
+}
+
 static u32 a618_get_speed_bin(u32 fuse)
 {
if (fuse == 0)
@@ -2195,6 +2219,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct 
adreno_gpu *adreno_gpu, u3
 {
u32 val = UINT_MAX;
 
+   if (adreno_is_a610(adreno_gpu))
+   val = a610_get_speed_bin(fuse);
+
if (adreno_is_a618(adreno_gpu))
val = a618_get_speed_bin(fuse);
 

-- 
2.40.1



[Freedreno] [PATCH v8 14/18] drm/msm/a6xx: Fix some A619 tunables

2023-05-29 Thread Konrad Dybcio
Adreno 619 expects some tunables to be set differently. Make up for it.

Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c0d5973320d9..1a29e7dd9975 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1198,6 +1198,8 @@ static int hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
+   else if (adreno_is_a619(adreno_gpu))
+   gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00018000);
else if (adreno_is_a610(adreno_gpu))
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x0008);
else
@@ -1215,7 +1217,9 @@ static int hw_init(struct msm_gpu *gpu)
a6xx_set_ubwc_config(gpu);
 
/* Enable fault detection */
-   if (adreno_is_a610(adreno_gpu))
+   if (adreno_is_a619(adreno_gpu))
+   gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) 
| 0x3f);
+   else if (adreno_is_a610(adreno_gpu))
gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) 
| 0x3);
else
gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30) 
| 0x1f);

-- 
2.40.1



[Freedreno] [PATCH v8 12/18] drm/msm/a6xx: Add support for A619_holi

2023-05-29 Thread Konrad Dybcio
A619_holi is a GMU-less variant of the already-supported A619 GPU.
It's present on at least SM4350 (holi) and SM6375 (blair). No mesa
changes are required. Add the required kernel-side support for it.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 27 +--
 drivers/gpu/drm/msm/adreno/adreno_gpu.h |  5 +
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 0a44762dbb6d..bb04f65e6f68 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -810,6 +810,9 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
if (adreno_is_a618(adreno_gpu))
return;
 
+   if (adreno_is_a619_holi(adreno_gpu))
+   hbb_lo = 0;
+
if (adreno_is_a640_family(adreno_gpu))
amsbc = 1;
 
@@ -1027,7 +1030,12 @@ static int hw_init(struct msm_gpu *gpu)
}
 
/* Clear GBIF halt in case GX domain was not collapsed */
-   if (a6xx_has_gbif(adreno_gpu)) {
+   if (adreno_is_a619_holi(adreno_gpu)) {
+   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
+   gpu_write(gpu, REG_A6XX_RBBM_GPR0_CNTL, 0);
+   /* Let's make extra sure that the GPU can access the memory.. */
+   mb();
+   } else if (a6xx_has_gbif(adreno_gpu)) {
gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 0);
/* Let's make extra sure that the GPU can access the memory.. */
@@ -1036,6 +1044,9 @@ static int hw_init(struct msm_gpu *gpu)
 
gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
 
+   if (adreno_is_a619_holi(adreno_gpu))
+   a6xx_sptprac_enable(gmu);
+
/*
 * Disable the trusted memory range - we don't actually supported secure
 * memory rendering at this point in time and we don't want to block off
@@ -1656,12 +1667,18 @@ static void a6xx_llc_slices_init(struct platform_device 
*pdev,
 #define GBIF_CLIENT_HALT_MASK  BIT(0)
 #define GBIF_ARB_HALT_MASK BIT(1)
 #define VBIF_XIN_HALT_CTRL0_MASK   GENMASK(3, 0)
+#define VBIF_RESET_ACK_MASK0xF0
+#define GPR0_GBIF_HALT_REQUEST 0x1E0
 
 void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool 
gx_off)
 {
struct msm_gpu *gpu = &adreno_gpu->base;
 
-   if (!a6xx_has_gbif(adreno_gpu)) {
+   if (adreno_is_a619_holi(adreno_gpu)) {
+   gpu_write(gpu, 0x18, GPR0_GBIF_HALT_REQUEST);
+   spin_until((gpu_read(gpu, REG_A6XX_RBBM_VBIF_GX_RESET_STATUS) &
+   (VBIF_RESET_ACK_MASK)) == VBIF_RESET_ACK_MASK);
+   } else if (!a6xx_has_gbif(adreno_gpu)) {
gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 
VBIF_XIN_HALT_CTRL0_MASK);
spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) &
(VBIF_XIN_HALT_CTRL0_MASK)) == 
VBIF_XIN_HALT_CTRL0_MASK);
@@ -1756,6 +1773,9 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
if (ret)
goto err_bulk_clk;
 
+   if (adreno_is_a619_holi(adreno_gpu))
+   a6xx_sptprac_enable(gmu);
+
/* If anything goes south, tear the GPU down piece by piece.. */
if (ret) {
 err_bulk_clk:
@@ -1815,6 +1835,9 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
/* Drain the outstanding traffic on memory buses */
a6xx_bus_clear_pending_transactions(adreno_gpu, true);
 
+   if (adreno_is_a619_holi(adreno_gpu))
+   a6xx_sptprac_disable(gmu);
+
clk_bulk_disable_unprepare(gpu->nr_clocks, gpu->grp_clks);
 
pm_runtime_put_sync(gmu->gxpd);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index ee5352bc5329..432fee5c1516 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -252,6 +252,11 @@ static inline int adreno_is_a619(struct adreno_gpu *gpu)
return gpu->revn == 619;
 }
 
+static inline int adreno_is_a619_holi(struct adreno_gpu *gpu)
+{
+   return adreno_is_a619(gpu) && adreno_has_gmu_wrapper(gpu);
+}
+
 static inline int adreno_is_a630(struct adreno_gpu *gpu)
 {
return gpu->revn == 630;

-- 
2.40.1



[Freedreno] [PATCH v8 11/18] drm/msm/adreno: Disable has_cached_coherent in GMU wrapper configurations

2023-05-29 Thread Konrad Dybcio
A610 and A619_holi don't support the feature. Disable it to make the GPU stop
crashing after almost each and every submission - the received data on
the GPU end was simply incomplete in garbled, resulting in almost nothing
being executed properly. Extend the disablement to adreno_has_gmu_wrapper,
as none of the GMU wrapper Adrenos that don't support yet seem to feature it.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 8cff86e9d35c..b133755a56c4 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -551,7 +551,6 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
config.rev.minor, config.rev.patchid);
 
priv->is_a2xx = config.rev.core == 2;
-   priv->has_cached_coherent = config.rev.core >= 6;
 
gpu = info->init(drm);
if (IS_ERR(gpu)) {
@@ -563,6 +562,10 @@ static int adreno_bind(struct device *dev, struct device 
*master, void *data)
if (ret)
return ret;
 
+   if (config.rev.core >= 6)
+   if (!adreno_has_gmu_wrapper(to_adreno_gpu(gpu)))
+   priv->has_cached_coherent = true;
+
return 0;
 }
 

-- 
2.40.1



[Freedreno] [PATCH v8 10/18] drm/msm/a6xx: Introduce GMU wrapper support

2023-05-29 Thread Konrad Dybcio
Some (particularly SMD_RPM, a.k.a non-RPMh) SoCs implement A6XX GPUs
but don't implement the associated GMUs. This is due to the fact that
the GMU directly pokes at RPMh. Sadly, this means we have to take care
of enabling & scaling power rails, clocks and bandwidth ourselves.

Reuse existing Adreno-common code and modify the deeply-GMU-infused
A6XX code to facilitate these GPUs. This involves if-ing out lots
of GMU callbacks and introducing a new type of GMU - GMU wrapper (it's
the actual name that Qualcomm uses in their downstream kernels).

This is essentially a register region which is convenient to model
as a device. We'll use it for managing the GDSCs. The register
layout matches the actual GMU_CX/GX regions on the "real GMU" devices
and lets us reuse quite a bit of gmu_read/write/rmw calls.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c   |  72 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c   | 211 
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h   |   1 +
 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |  14 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c |   8 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.h |   6 +
 6 files changed, 277 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 5ba8cba69383..385ca3a12462 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1437,6 +1437,7 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, struct 
platform_device *pdev,
 
 void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
 {
+   struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
struct platform_device *pdev = to_platform_device(gmu->dev);
 
@@ -1462,10 +1463,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
gmu->mmio = NULL;
gmu->rscc = NULL;
 
-   a6xx_gmu_memory_free(gmu);
+   if (!adreno_has_gmu_wrapper(adreno_gpu)) {
+   a6xx_gmu_memory_free(gmu);
 
-   free_irq(gmu->gmu_irq, gmu);
-   free_irq(gmu->hfi_irq, gmu);
+   free_irq(gmu->gmu_irq, gmu);
+   free_irq(gmu->hfi_irq, gmu);
+   }
 
/* Drop reference taken in of_find_device_by_node */
put_device(gmu->dev);
@@ -1484,6 +1487,69 @@ static int cxpd_notifier_cb(struct notifier_block *nb,
return 0;
 }
 
+int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
+{
+   struct platform_device *pdev = of_find_device_by_node(node);
+   struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
+   int ret;
+
+   if (!pdev)
+   return -ENODEV;
+
+   gmu->dev = &pdev->dev;
+
+   of_dma_configure(gmu->dev, node, true);
+
+   pm_runtime_enable(gmu->dev);
+
+   /* Mark legacy for manual SPTPRAC control */
+   gmu->legacy = true;
+
+   /* Map the GMU registers */
+   gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
+   if (IS_ERR(gmu->mmio)) {
+   ret = PTR_ERR(gmu->mmio);
+   goto err_mmio;
+   }
+
+   gmu->cxpd = dev_pm_domain_attach_by_name(gmu->dev, "cx");
+   if (IS_ERR(gmu->cxpd)) {
+   ret = PTR_ERR(gmu->cxpd);
+   goto err_mmio;
+   }
+
+   if (!device_link_add(gmu->dev, gmu->cxpd, DL_FLAG_PM_RUNTIME)) {
+   ret = -ENODEV;
+   goto detach_cxpd;
+   }
+
+   init_completion(&gmu->pd_gate);
+   complete_all(&gmu->pd_gate);
+   gmu->pd_nb.notifier_call = cxpd_notifier_cb;
+
+   /* Get a link to the GX power domain to reset the GPU */
+   gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
+   if (IS_ERR(gmu->gxpd)) {
+   ret = PTR_ERR(gmu->gxpd);
+   goto err_mmio;
+   }
+
+   gmu->initialized = true;
+
+   return 0;
+
+detach_cxpd:
+   dev_pm_domain_detach(gmu->cxpd, false);
+
+err_mmio:
+   iounmap(gmu->mmio);
+
+   /* Drop reference taken in of_find_device_by_node */
+   put_device(gmu->dev);
+
+   return ret;
+}
+
 int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 {
struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 58bf405b85d8..0a44762dbb6d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -21,7 +21,7 @@ static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
 
/* Check that the GMU is idle */
-   if (!a6xx_gmu_isidle(&a6xx_gpu->gmu))
+   if (!adreno_has_gmu_wrapper(adreno_gpu) && 
!a6xx_gmu_isidle(&a6xx_gpu->gmu))
return false;
 
/* Check tha the CX master is idle */
@@ -1018,10 +1018,13 @@ static int hw_init(struct msm_gpu *gpu)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a

[Freedreno] [PATCH v8 08/18] drm/msm/a6xx: Remove both GBIF and RBBM GBIF halt on hw init

2023-05-29 Thread Konrad Dybcio
Currently we're only deasserting REG_A6XX_RBBM_GBIF_HALT, but we also
need REG_A6XX_GBIF_HALT to be set to 0.

This is typically done automatically on successful GX collapse, but in
case that fails, we should take care of it.

Also, add a memory barrier to ensure it's gone through before jumping
to further initialization.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 083ccb5bcb4e..dfde5fb65eed 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1003,8 +1003,12 @@ static int hw_init(struct msm_gpu *gpu)
a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_GPU_SET);
 
/* Clear GBIF halt in case GX domain was not collapsed */
-   if (a6xx_has_gbif(adreno_gpu))
+   if (a6xx_has_gbif(adreno_gpu)) {
+   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 0);
+   /* Let's make extra sure that the GPU can access the memory.. */
+   mb();
+   }
 
gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
 

-- 
2.40.1



[Freedreno] [PATCH v8 07/18] drm/msm/a6xx: Add a helper for software-resetting the GPU

2023-05-29 Thread Konrad Dybcio
Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper
GPUs and reuse it in a6xx_gmu_force_off().

This helper, contrary to the original usage in GMU code paths, adds
a write memory barrier which together with the necessary delay should
ensure that the reset is never deasserted too quickly due to e.g. OoO
execution going crazy.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c |  3 +--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h |  1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index b86be123ecd0..5ba8cba69383 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
a6xx_bus_clear_pending_transactions(adreno_gpu, true);
 
/* Reset GPU core blocks */
-   gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1);
-   udelay(100);
+   a6xx_gpu_sw_reset(gpu, true);
 }
 
 static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu 
*gmu)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e3ac3f045665..083ccb5bcb4e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct 
adreno_gpu *adreno_gpu, bool gx_
gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
 }
 
+void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert)
+{
+   gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert);
+   /* Add a barrier to avoid bad surprises */
+   mb();
+
+   /* The reset line needs to be asserted for at least 100 us */
+   if (assert)
+   udelay(100);
+}
+
 static int a6xx_pm_resume(struct msm_gpu *gpu)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index 9580def06d45..aa70390ee1c6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu);
 int a6xx_gpu_state_put(struct msm_gpu_state *state);
 
 void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool 
gx_off);
+void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert);
 
 #endif /* __A6XX_GPU_H__ */

-- 
2.40.1



[Freedreno] [PATCH v8 09/18] drm/msm/a6xx: Extend and explain UBWC config

2023-05-29 Thread Konrad Dybcio
Rename lower_bit to hbb_lo and explain what it signifies.
Add explanations (wherever possible to other tunables).

Port setting min_access_length, ubwc_mode and hbb_hi from downstream.

Reviewed-by: Rob Clark 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 39 +++
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index dfde5fb65eed..58bf405b85d8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -786,10 +786,25 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
 static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
-   u32 lower_bit = 2;
-   u32 amsbc = 0;
+   /* Unknown, introduced with A650 family, related to UBWC mode/ver 4 */
u32 rgb565_predicator = 0;
+   /* Unknown, introduced with A650 family */
u32 uavflagprd_inv = 0;
+   /* Whether the minimum access length is 64 bits */
+   u32 min_acc_len = 0;
+   /* Entirely magic, per-GPU-gen value */
+   u32 ubwc_mode = 0;
+   /*
+* The Highest Bank Bit value represents the bit of the highest DDR 
bank.
+* We then subtract 13 from it (13 is the minimum value allowed by hw) 
and
+* write the lowest two bits of the remaining value as hbb_lo and the
+* one above it as hbb_hi to the hardware. This should ideally use DRAM
+* type detection.
+*/
+   u32 hbb_hi = 0;
+   u32 hbb_lo = 2;
+   /* Unknown, introduced with A640/680 */
+   u32 amsbc = 0;
 
/* a618 is using the hw default values */
if (adreno_is_a618(adreno_gpu))
@@ -800,25 +815,31 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
 
if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu)) {
/* TODO: get ddr type from bootloader and use 2 for LPDDR4 */
-   lower_bit = 3;
+   hbb_lo = 3;
amsbc = 1;
rgb565_predicator = 1;
uavflagprd_inv = 2;
}
 
if (adreno_is_7c3(adreno_gpu)) {
-   lower_bit = 1;
+   hbb_lo = 1;
amsbc = 1;
rgb565_predicator = 1;
uavflagprd_inv = 2;
}
 
gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,
-   rgb565_predicator << 11 | amsbc << 4 | lower_bit << 1);
-   gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, lower_bit << 1);
-   gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL,
-   uavflagprd_inv << 4 | lower_bit << 1);
-   gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, lower_bit << 21);
+ rgb565_predicator << 11 | hbb_hi << 10 | amsbc << 4 |
+ min_acc_len << 3 | hbb_lo << 1 | ubwc_mode);
+
+   gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, hbb_hi << 4 |
+ min_acc_len << 3 | hbb_lo << 1 | ubwc_mode);
+
+   gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, hbb_hi << 10 |
+ uavflagprd_inv << 4 | min_acc_len << 3 |
+ hbb_lo << 1 | ubwc_mode);
+
+   gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, min_acc_len << 23 | hbb_lo << 
21);
 }
 
 static int a6xx_cp_init(struct msm_gpu *gpu)

-- 
2.40.1



[Freedreno] [PATCH v8 06/18] drm/msm/a6xx: Improve a6xx_bus_clear_pending_transactions()

2023-05-29 Thread Konrad Dybcio
Unify the indentation and explain the cryptic 0xF value.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 6bb4da70f6a6..e3ac3f045665 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1597,17 +1597,18 @@ static void a6xx_llc_slices_init(struct platform_device 
*pdev,
a6xx_gpu->llc_mmio = ERR_PTR(-EINVAL);
 }
 
-#define GBIF_CLIENT_HALT_MASK BIT(0)
-#define GBIF_ARB_HALT_MASKBIT(1)
+#define GBIF_CLIENT_HALT_MASK  BIT(0)
+#define GBIF_ARB_HALT_MASK BIT(1)
+#define VBIF_XIN_HALT_CTRL0_MASK   GENMASK(3, 0)
 
 void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool 
gx_off)
 {
struct msm_gpu *gpu = &adreno_gpu->base;
 
if (!a6xx_has_gbif(adreno_gpu)) {
-   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0xf);
+   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 
VBIF_XIN_HALT_CTRL0_MASK);
spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) &
-   0xf) == 0xf);
+   (VBIF_XIN_HALT_CTRL0_MASK)) == 
VBIF_XIN_HALT_CTRL0_MASK);
gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0);
 
return;

-- 
2.40.1



[Freedreno] [PATCH v8 04/18] drm/msm/a6xx: Move force keepalive vote removal to a6xx_gmu_force_off()

2023-05-29 Thread Konrad Dybcio
As pointed out by Akhil during the review process of GMU wrapper
introduction [1], it makes sense to move this write into the function
that's responsible for forcibly shutting the GMU off.

It is also very convenient to move this to GMU-specific code, so that
it does not have to be guarded by an if-condition to avoid calling it
on GMU wrapper targets.

Move the write to the aforementioned a6xx_gmu_force_off() to achieve
that. No effective functional change.

[1] 
https://lore.kernel.org/linux-arm-msm/20230501194022.ga18...@akhilpo-linux.qualcomm.com/
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 ++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 --
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 87babbb2a19f..9421716a2fe5 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -912,6 +912,12 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
struct msm_gpu *gpu = &adreno_gpu->base;
 
+   /*
+* Turn off keep alive that might have been enabled by the hang
+* interrupt
+*/
+   gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 0);
+
/* Flush all the queues */
a6xx_hfi_stop(gmu);
 
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 9fb214f150dd..e34aa15156a4 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1274,12 +1274,6 @@ static void a6xx_recover(struct msm_gpu *gpu)
/* Halt SQE first */
gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 3);
 
-   /*
-* Turn off keep alive that might have been enabled by the hang
-* interrupt
-*/
-   gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_GMU_PWR_COL_KEEPALIVE, 0);
-
pm_runtime_dont_use_autosuspend(&gpu->pdev->dev);
 
/* active_submit won't change until we make a submission */

-- 
2.40.1



[Freedreno] [PATCH v8 03/18] drm/msm/a6xx: Remove static keyword from sptprac en/disable functions

2023-05-29 Thread Konrad Dybcio
These two will be reused by at least A619_holi in the non-gmu
paths. Turn them non-static them to make it possible.

Reviewed-by: Dmitry Baryshkov 
Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 ++--
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e16b4b3f8535..87babbb2a19f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -354,7 +354,7 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum 
a6xx_gmu_oob_state state)
 }
 
 /* Enable CPU control of SPTP power power collapse */
-static int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
+int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
 {
int ret;
u32 val;
@@ -376,7 +376,7 @@ static int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
 }
 
 /* Disable CPU control of SPTP power power collapse */
-static void a6xx_sptprac_disable(struct a6xx_gmu *gmu)
+void a6xx_sptprac_disable(struct a6xx_gmu *gmu)
 {
u32 val;
int ret;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 0bc3eb443fec..7ee5b606bc47 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -193,5 +193,7 @@ int a6xx_hfi_set_freq(struct a6xx_gmu *gmu, int index);
 
 bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu);
 bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu);
+void a6xx_sptprac_disable(struct a6xx_gmu *gmu);
+int a6xx_sptprac_enable(struct a6xx_gmu *gmu);
 
 #endif

-- 
2.40.1



[Freedreno] [PATCH v8 05/18] drm/msm/a6xx: Move a6xx_bus_clear_pending_transactions to a6xx_gpu

2023-05-29 Thread Konrad Dybcio
This function is responsible for telling the GPU to halt transactions
on all of its relevant buses, drain them and leave them in a predictable
state, so that the GPU can be e.g. reset cleanly.

Move the function to a6xx_gpu.c, remove the static keyword and add a
prototype in a6xx_gpu.h to accomodate for the move.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 37 ---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 36 ++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h |  2 ++
 3 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 9421716a2fe5..b86be123ecd0 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -868,43 +868,6 @@ static void a6xx_gmu_rpmh_off(struct a6xx_gmu *gmu)
(val & 1), 100, 1000);
 }
 
-#define GBIF_CLIENT_HALT_MASK BIT(0)
-#define GBIF_ARB_HALT_MASKBIT(1)
-
-static void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu,
-   bool gx_off)
-{
-   struct msm_gpu *gpu = &adreno_gpu->base;
-
-   if (!a6xx_has_gbif(adreno_gpu)) {
-   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0xf);
-   spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) &
-   0xf) == 0xf);
-   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0);
-
-   return;
-   }
-
-   if (gx_off) {
-   /* Halt the gx side of GBIF */
-   gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 1);
-   spin_until(gpu_read(gpu, REG_A6XX_RBBM_GBIF_HALT_ACK) & 1);
-   }
-
-   /* Halt new client requests on GBIF */
-   gpu_write(gpu, REG_A6XX_GBIF_HALT, GBIF_CLIENT_HALT_MASK);
-   spin_until((gpu_read(gpu, REG_A6XX_GBIF_HALT_ACK) &
-   (GBIF_CLIENT_HALT_MASK)) == GBIF_CLIENT_HALT_MASK);
-
-   /* Halt all AXI requests on GBIF */
-   gpu_write(gpu, REG_A6XX_GBIF_HALT, GBIF_ARB_HALT_MASK);
-   spin_until((gpu_read(gpu,  REG_A6XX_GBIF_HALT_ACK) &
-   (GBIF_ARB_HALT_MASK)) == GBIF_ARB_HALT_MASK);
-
-   /* The GBIF halt needs to be explicitly cleared */
-   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
-}
-
 /* Force the GMU off in case it isn't responsive */
 static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
 {
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e34aa15156a4..6bb4da70f6a6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1597,6 +1597,42 @@ static void a6xx_llc_slices_init(struct platform_device 
*pdev,
a6xx_gpu->llc_mmio = ERR_PTR(-EINVAL);
 }
 
+#define GBIF_CLIENT_HALT_MASK BIT(0)
+#define GBIF_ARB_HALT_MASKBIT(1)
+
+void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool 
gx_off)
+{
+   struct msm_gpu *gpu = &adreno_gpu->base;
+
+   if (!a6xx_has_gbif(adreno_gpu)) {
+   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0xf);
+   spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) &
+   0xf) == 0xf);
+   gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0);
+
+   return;
+   }
+
+   if (gx_off) {
+   /* Halt the gx side of GBIF */
+   gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 1);
+   spin_until(gpu_read(gpu, REG_A6XX_RBBM_GBIF_HALT_ACK) & 1);
+   }
+
+   /* Halt new client requests on GBIF */
+   gpu_write(gpu, REG_A6XX_GBIF_HALT, GBIF_CLIENT_HALT_MASK);
+   spin_until((gpu_read(gpu, REG_A6XX_GBIF_HALT_ACK) &
+   (GBIF_CLIENT_HALT_MASK)) == GBIF_CLIENT_HALT_MASK);
+
+   /* Halt all AXI requests on GBIF */
+   gpu_write(gpu, REG_A6XX_GBIF_HALT, GBIF_ARB_HALT_MASK);
+   spin_until((gpu_read(gpu,  REG_A6XX_GBIF_HALT_ACK) &
+   (GBIF_ARB_HALT_MASK)) == GBIF_ARB_HALT_MASK);
+
+   /* The GBIF halt needs to be explicitly cleared */
+   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
+}
+
 static int a6xx_pm_resume(struct msm_gpu *gpu)
 {
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index eea2e60ce3b7..9580def06d45 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -88,4 +88,6 @@ void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state 
*state,
 struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu);
 int a6xx_gpu_state_put(struct msm_gpu_state *state);
 
+void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool 
gx_off);
+
 #endif /* __A6XX_GPU_H__ */

-- 
2.40.1



[Freedreno] [PATCH v8 02/18] dt-bindings: display/msm/gmu: Add GMU wrapper

2023-05-29 Thread Konrad Dybcio
The "GMU Wrapper" is Qualcomm's name for "let's treat the GPU blocks
we'd normally assign to the GMU as if they were a part of the GMU, even
though they are not". It's a (good) software representation of the GMU_CX
and GMU_GX register spaces within the GPUSS that helps us programatically
treat these de-facto GMU-less parts in a way that's very similar to their
GMU-equipped cousins, massively saving up on code duplication.

The "wrapper" register space was specifically designed to mimic the layout
of a real GMU, though it rather obviously does not have the M3 core et al.

To sum it all up, the GMU wrapper is essentially a register space within
the GPU, which Linux sees as a dumbed-down regular GMU: there's no clocks,
interrupts, multiple reg spaces, iommus and OPP. Document it.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Konrad Dybcio 
---
 .../devicetree/bindings/display/msm/gmu.yaml   | 50 --
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml 
b/Documentation/devicetree/bindings/display/msm/gmu.yaml
index f31a26305ca9..5fc4106110ad 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
@@ -19,16 +19,18 @@ description: |
 
 properties:
   compatible:
-items:
-  - pattern: '^qcom,adreno-gmu-6[0-9][0-9]\.[0-9]$'
-  - const: qcom,adreno-gmu
+oneOf:
+  - items:
+  - pattern: '^qcom,adreno-gmu-6[0-9][0-9]\.[0-9]$'
+  - const: qcom,adreno-gmu
+  - const: qcom,adreno-gmu-wrapper
 
   reg:
-minItems: 3
+minItems: 1
 maxItems: 4
 
   reg-names:
-minItems: 3
+minItems: 1
 maxItems: 4
 
   clocks:
@@ -44,7 +46,6 @@ properties:
   - description: GMU HFI interrupt
   - description: GMU interrupt
 
-
   interrupt-names:
 items:
   - const: hfi
@@ -72,14 +73,8 @@ required:
   - compatible
   - reg
   - reg-names
-  - clocks
-  - clock-names
-  - interrupts
-  - interrupt-names
   - power-domains
   - power-domain-names
-  - iommus
-  - operating-points-v2
 
 additionalProperties: false
 
@@ -218,6 +213,28 @@ allOf:
 - const: axi
 - const: memnoc
 
+  - if:
+  properties:
+compatible:
+  contains:
+const: qcom,adreno-gmu-wrapper
+then:
+  properties:
+reg:
+  items:
+- description: GMU wrapper register space
+reg-names:
+  items:
+- const: gmu
+else:
+  required:
+- clocks
+- clock-names
+- interrupts
+- interrupt-names
+- iommus
+- operating-points-v2
+
 examples:
   - |
 #include 
@@ -250,3 +267,12 @@ examples:
 iommus = <&adreno_smmu 5>;
 operating-points-v2 = <&gmu_opp_table>;
 };
+
+gmu_wrapper: gmu@596a000 {
+compatible = "qcom,adreno-gmu-wrapper";
+reg = <0x0596a000 0x3>;
+reg-names = "gmu";
+power-domains = <&gpucc GPU_CX_GDSC>,
+<&gpucc GPU_GX_GDSC>;
+power-domain-names = "cx", "gx";
+};

-- 
2.40.1



[Freedreno] [PATCH v8 00/18] GMU-less A6xx support (A610, A619_holi)

2023-05-29 Thread Konrad Dybcio
v7 -> v8:
- Fix up resume/suspend (icc now correctly parks to 0, don't abuse
  OPP & genpd throughout system-wide suspend)
- Don't handle ebi1_clk separately, the bulk ops handle it just fine
- Rebase on next-20230525 (no meaningful changes)

v7: 
https://lore.kernel.org/linux-arm-msm/20230223-topic-gmuwrapper-v7-0-ecc7aab83...@linaro.org/

v6 -> v7:
- Rebase on next-20230519 (A640/650 speedbin merged already)

- separate out the .get_timestamp cb for gmu wrapper

- check for gmu presence inside a6xx_llc_slices_(init|destroy) instead
  of before calling them

- use REG_A6XX_RBBM_GPR0_CNTL instead of literal 0x18

- move a6xx_bus_clear_pending_transactions to a6xx_gpu, clean it up
  and reuse it for gmu wrapper gpus

- drop clearing RBBM_GBIF (GBIF from GX's POV) as part of draining the
  buses, it's not necessary

- introduce a helper for gpu softreset

- sw-reset the gmu wrapper GPUS *after* draining GBIF and only reset
  it if it's hung

- reword the commit message in "Remove both GBIF and RBBM GBIF halt
  on hw init" and move it before gmu wrapper-specific changes

- drop set_rate logic from a6xx_pm_suspend as the clock simply gets
  disabled and we don't have to worry about scaling problems as OPP
  and devfreq take care of that, validated with debugcc

- drop a level of indentation in _a6xx_check_idle() to hopefully
  improve readability

- check for !a610 instead of gmu_wrapper||a619_holi in sptprac cc
  toggling in a6xx_set_hwcg()

- pick up krzk's rb on bindings

All external dependencies have been merged since the last revision.

v6: 
https://lore.kernel.org/r/20230223-topic-gmuwrapper-v6-0-2034115bb...@linaro.org

v5 -> v6:
- Rebase on 8ead96783163 ("drm/msm/gpu: Move BO allocation out of hw_init")
  (Add .ucode_load to funcs_gmuwrapper)
- Drop A6[45]0 speedbin deps, merged into msm-next

Dependencies:
- 
https://lore.kernel.org/linux-arm-msm/20230330231517.2747024-1-konrad.dyb...@linaro.org/
 (to work properly)

v5: 
https://lore.kernel.org/linux-arm-msm/20230223-topic-gmuwrapper-v5-0-bf774b9a9...@linaro.org/

v4 -> v5:
- Add a newline before the new allOf:if: [3/15]
- Enforce 6 clocks on A619_holi/A610 [2/15]
- Pick up tags
- Improve error handling in a6xx_pm_resume [6/15]
- Add patch [1/15] (fix an existing issue) which can be picked
  separately and account for it in [6/15]
- Rebase atop Akhil's CX shutdown patches and incorporate analogous logic
- Fix a regression introduced in v3 that made the fw loader expect
  GMU fw on GMU wrapper GPUs

Dependencies:
- 
https://lore.kernel.org/linux-arm-msm/20230120172233.1905761-1-konrad.dyb...@linaro.org/
 (to apply)
- 
https://lore.kernel.org/linux-arm-msm/20230330231517.2747024-1-konrad.dyb...@linaro.org/
 (to work properly)

v4: 
https://lore.kernel.org/r/20230223-topic-gmuwrapper-v4-0-e987eb79d...@linaro.org

v3 -> v4:
- Drop the mistakengly-included and wrong A3xx-A5xx bindings changes
- Improve bindings commit messages to better explain what GMU Wrapper is
- Drop the A680 highest bank bit value adjustment patch
- Sort UBWC config variables in a reverse-Christmass-tree fashion [4/14]
- Don't alter any UBWC config values in [4/14]
  - Do so for a619_holi in [8/14]
- Rebase on next-20230314 (shouldn't matter at all)

v3: 
https://lore.kernel.org/r/20230223-topic-gmuwrapper-v3-0-5be55a336...@linaro.org

v2 -> v3:
New dependencies:
- 
https://lore.kernel.org/linux-arm-msm/20230223-topic-opp-v3-0-5f22163cd...@linaro.org/T/#t
- 
https://lore.kernel.org/linux-arm-msm/20230120172233.1905761-1-konrad.dyb...@linaro.org/

Sidenote: A speedbin rework is in progress, the of_machine_is_compatible
calls in A619_holi are ugly (but well, necessary..) but they'll be
replaced with socid matching in this or the next kernel cycle.

Due to the new way of identifying GMU wrapper GPUs, configuring 6350
to use wrapper would cause the wrong fuse values to be checked, but that
will be solved by the conversion + the ultimate goal is to use the GMU
whenever possible with the wrapper left for GMU-less Adrenos and early
bringup debugging of GMU-equipped ones.

- Ship dt-bindings in this series as we're referencing the compatible now

- "De-staticize" -> "remove static keyword" [3/15]

- Track down all the values in [4/15]

- Add many comments and explanations in [4/15]

- Fix possible return-before-mutex-unlock [5/15]

- Explain the GMU wrapper a bit more in the commit msg [5/15]

- Separate out pm_resume/suspend for GMU-wrapper GPUs to make things
  cleaner [5/15]

- Don't check if `info` exists, it has to at this point [5/15]

- Assign gpu->info early and clean up following if statements in
  a6xx_gpu_init [5/15]

- Determine whether we use GMU wrapper based on the GMU compatible
  instead of a quirk [5/15]

- Use a struct field to annotate whether we're using gmu wrapper so
  that it can be assigned at runtime (turns out a619 holi-ness cannot
  be determined by patchid + that will make it easier to test out GMU
  GPUs without actually turning on the GMU if anybody wants to d

[Freedreno] [PATCH v8 01/18] dt-bindings: display/msm: gpu: Document GMU wrapper-equipped A6xx

2023-05-29 Thread Konrad Dybcio
The "GMU Wrapper" is Qualcomm's name for "let's treat the GPU blocks
we'd normally assign to the GMU as if they were a part of the GMU, even
though they are not". It's a (good) software representation of the GMU_CX
and GMU_GX register spaces within the GPUSS that helps us programatically
treat these de-facto GMU-less parts in a way that's very similar to their
GMU-equipped cousins, massively saving up on code duplication.

The "wrapper" register space was specifically designed to mimic the layout
of a real GMU, though it rather obviously does not have the M3 core et al.

GMU wrapper-equipped A6xx GPUs require clocks and clock-names to be
specified under the GPU node, just like their older cousins. Account
for that.

Signed-off-by: Konrad Dybcio 
---
 .../devicetree/bindings/display/msm/gpu.yaml   | 61 ++
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml 
b/Documentation/devicetree/bindings/display/msm/gpu.yaml
index 5dabe7b6794b..58ca8912a8c3 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -36,10 +36,7 @@ properties:
 
   reg-names:
 minItems: 1
-items:
-  - const: kgsl_3d0_reg_memory
-  - const: cx_mem
-  - const: cx_dbgc
+maxItems: 3
 
   interrupts:
 maxItems: 1
@@ -157,16 +154,62 @@ allOf:
   required:
 - clocks
 - clock-names
+
   - if:
   properties:
 compatible:
   contains:
-pattern: '^qcom,adreno-6[0-9][0-9]\.[0-9]$'
-
-then: # Since Adreno 6xx series clocks should be defined in GMU
+enum:
+  - qcom,adreno-610.0
+  - qcom,adreno-619.1
+then:
   properties:
-clocks: false
-clock-names: false
+clocks:
+  minItems: 6
+  maxItems: 6
+
+clock-names:
+  items:
+- const: core
+  description: GPU Core clock
+- const: iface
+  description: GPU Interface clock
+- const: mem_iface
+  description: GPU Memory Interface clock
+- const: alt_mem_iface
+  description: GPU Alternative Memory Interface clock
+- const: gmu
+  description: CX GMU clock
+- const: xo
+  description: GPUCC clocksource clock
+
+reg-names:
+  minItems: 1
+  items:
+- const: kgsl_3d0_reg_memory
+- const: cx_dbgc
+
+  required:
+- clocks
+- clock-names
+else:
+  if:
+properties:
+  compatible:
+contains:
+  pattern: '^qcom,adreno-6[0-9][0-9]\.[0-9]$'
+
+  then: # Starting with A6xx, the clocks are usually defined in the GMU 
node
+properties:
+  clocks: false
+  clock-names: false
+
+  reg-names:
+minItems: 1
+items:
+  - const: kgsl_3d0_reg_memory
+  - const: cx_mem
+  - const: cx_dbgc
 
 examples:
   - |

-- 
2.40.1



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

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 14:59, Konrad Dybcio wrote:



On 29.05.2023 11:44, Luca Weiss wrote:

Add the required config for the v1.1 MDP5 found on MSM8226.

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 = {

Luca, this patch looks good as-is (without diving into the values).

Dmitry, I see some things that we may improve here..

1. Rename msm8xab to msm89ab or something, it's really inconsistent
with other drivers

2. Some values seem very common / always constant.. perhaps we could
add some #defines like we do in DPU?


I really would not like to go the DPU way here. Maybe we can define the 
'full-featured' masks, while leaving the older hardware intact.



3. Can we add some magic defines to make flush_hw_mask non-cryptic?


Sounds like a good idea, especially since we have all the defines. I'll 
tend to it after landing 8226.




4. We can probably use pointers in data structs and deduplicate identical
blocks!


Let's see.



Konrad

+   .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 = &msm8x74v1_config } },
+   { .revision = 1, .config = { .hw = &msm8x26_config } },
{ .revision = 2, .config = { .hw = &msm8x74v2_config } },
{ .revision = 3, .config = { .hw = &apq8084_config } },
{ .revision = 6, .config = { .hw = &msm8x16_config } },



--
With best wishes
Dmitry



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

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 12:44, Luca Weiss wrote:

Add the required config for the v1.1 MDP5 found on MSM8226.

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


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



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

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 14:19, Dmitry Baryshkov wrote:
> On 29/05/2023 15:10, Konrad Dybcio wrote:
>>
>>
>> On 29.05.2023 11:44, Luca Weiss wrote:
>>> 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 | 118 
>>> 
>>>   1 file changed, 118 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi 
>>> b/arch/arm/boot/dts/qcom-msm8226.dtsi
>>> index 42acb9ddb8cc..182d6405032f 100644
>>> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
>>> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
>>> @@ -636,6 +636,124 @@ smd-edge {
>>>   label = "lpass";
>>>   };
>>>   };
>>> +
>>> +    mdss: display-subsystem@fd90 {
>>> +    compatible = "qcom,mdss";
>>> +    reg = <0xfd90 0x100>, <0xfd924000 0x1000>;
>>> +    reg-names = "mdss_phys", "vbif_phys";
>>> +
>>> +    power-domains = <&mmcc MDSS_GDSC>;
>>> +
>>> +    clocks = <&mmcc MDSS_AHB_CLK>,
>>> + <&mmcc MDSS_AXI_CLK>,
>>> + <&mmcc MDSS_VSYNC_CLK>;
>>> +    clock-names = "iface", "bus", "vsync";
>> One per line, please
>>
>>> +
>>> +    interrupts = ;
>>> +
>>> +    interrupt-controller;
>>> +    #interrupt-cells = <1>;
>> We're not using the irq cell, is that necessary/should that be 0?
> 
> No. With 0 it would mean that there is a single interrupt for mdss source, 
> which clearly is not the case.
Obviously. Derp, sorry.

Konrad
> 
>>
>>> +
>>> +    status = "disabled";
>> status should go last
>>
>>> +
>>> +    #address-cells = <1>;
>>> +    #size-cells = <1>;
>>> +    ranges;
>>> +
>>> +    mdp: display-controller@fd90 {
>>> +    compatible = "qcom,msm8226-mdp5", "qcom,mdp5";
>>> +    reg = <0xfd900100 0x22000>;
>>> +    reg-names = "mdp_phys";
>>> +
>>> +    interrupt-parent = <&mdss>;
>>> +    interrupts = <0>;
>>> +
>>> +    clocks = <&mmcc MDSS_AHB_CLK>,
>>> + <&mmcc MDSS_AXI_CLK>,
>>> + <&mmcc MDSS_MDP_CLK>,
>>> + <&mmcc MDSS_VSYNC_CLK>;
>>> +    clock-names = "iface", "bus", "core", "vsync";
>> One per line, please
>>
>>> +
>>> +    ports {
>>> +    #address-cells = <1>;
>>> +    #size-cells = <0>;
>> Would port { work here? I remember one mdss component's bindings
>> didn't allow it but don't recall which one
> 
> Let's use ports /port@0 for uniformity even if there is just a single port 
> always.
> 
>>
>>> +
>>> +    port@0 {
>>> +    reg = <0>;
>>> +    mdp5_intf1_out: endpoint {
>>> +    remote-endpoint = <&dsi0_in>;
>>> +    };
>>> +    };
>>> +    };
>>> +    };
>>> +-- 
> With best wishes
> Dmitry
> 


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

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 15:10, Konrad Dybcio wrote:



On 29.05.2023 11:44, Luca Weiss wrote:

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 | 118 
  1 file changed, 118 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi 
b/arch/arm/boot/dts/qcom-msm8226.dtsi
index 42acb9ddb8cc..182d6405032f 100644
--- a/arch/arm/boot/dts/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
@@ -636,6 +636,124 @@ smd-edge {
label = "lpass";
};
};
+
+   mdss: display-subsystem@fd90 {
+   compatible = "qcom,mdss";
+   reg = <0xfd90 0x100>, <0xfd924000 0x1000>;
+   reg-names = "mdss_phys", "vbif_phys";
+
+   power-domains = <&mmcc MDSS_GDSC>;
+
+   clocks = <&mmcc MDSS_AHB_CLK>,
+<&mmcc MDSS_AXI_CLK>,
+<&mmcc MDSS_VSYNC_CLK>;
+   clock-names = "iface", "bus", "vsync";

One per line, please


+
+   interrupts = ;
+
+   interrupt-controller;
+   #interrupt-cells = <1>;

We're not using the irq cell, is that necessary/should that be 0?


No. With 0 it would mean that there is a single interrupt for mdss 
source, which clearly is not the case.





+
+   status = "disabled";

status should go last


+
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   mdp: display-controller@fd90 {
+   compatible = "qcom,msm8226-mdp5", "qcom,mdp5";
+   reg = <0xfd900100 0x22000>;
+   reg-names = "mdp_phys";
+
+   interrupt-parent = <&mdss>;
+   interrupts = <0>;
+
+   clocks = <&mmcc MDSS_AHB_CLK>,
+<&mmcc MDSS_AXI_CLK>,
+<&mmcc MDSS_MDP_CLK>,
+<&mmcc MDSS_VSYNC_CLK>;
+   clock-names = "iface", "bus", "core", "vsync";

One per line, please


+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;

Would port { work here? I remember one mdss component's bindings
didn't allow it but don't recall which one


Let's use ports /port@0 for uniformity even if there is just a single 
port always.





+
+   port@0 {
+   reg = <0>;
+   mdp5_intf1_out: endpoint {
+   remote-endpoint = 
<&dsi0_in>;
+   };
+   };
+   };
+   };
+-- 

With best wishes
Dmitry



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

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 12:44, Luca Weiss wrote:

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.

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(+)


Reviewed-by: Dmitry Baryshkov 

--
With best wishes
Dmitry



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

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 11:44, Luca Weiss wrote:
> 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 | 118 
> 
>  1 file changed, 118 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi 
> b/arch/arm/boot/dts/qcom-msm8226.dtsi
> index 42acb9ddb8cc..182d6405032f 100644
> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -636,6 +636,124 @@ smd-edge {
>   label = "lpass";
>   };
>   };
> +
> + mdss: display-subsystem@fd90 {
> + compatible = "qcom,mdss";
> + reg = <0xfd90 0x100>, <0xfd924000 0x1000>;
> + reg-names = "mdss_phys", "vbif_phys";
> +
> + power-domains = <&mmcc MDSS_GDSC>;
> +
> + clocks = <&mmcc MDSS_AHB_CLK>,
> +  <&mmcc MDSS_AXI_CLK>,
> +  <&mmcc MDSS_VSYNC_CLK>;
> + clock-names = "iface", "bus", "vsync";
One per line, please

> +
> + interrupts = ;
> +
> + interrupt-controller;
> + #interrupt-cells = <1>;
We're not using the irq cell, is that necessary/should that be 0?

> +
> + status = "disabled";
status should go last

> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + mdp: display-controller@fd90 {
> + compatible = "qcom,msm8226-mdp5", "qcom,mdp5";
> + reg = <0xfd900100 0x22000>;
> + reg-names = "mdp_phys";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <0>;
> +
> + clocks = <&mmcc MDSS_AHB_CLK>,
> +  <&mmcc MDSS_AXI_CLK>,
> +  <&mmcc MDSS_MDP_CLK>,
> +  <&mmcc MDSS_VSYNC_CLK>;
> + clock-names = "iface", "bus", "core", "vsync";
One per line, please

> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
Would port { work here? I remember one mdss component's bindings
didn't allow it but don't recall which one

> +
> + port@0 {
> + reg = <0>;
> + mdp5_intf1_out: endpoint {
> + remote-endpoint = 
> <&dsi0_in>;
> + };
> + };
> + };
> + };
> +
> + dsi0: dsi@fd922800 {
> + compatible = "qcom,msm8226-dsi-ctrl",
> +  "qcom,mdss-dsi-ctrl";
> + reg = <0xfd922800 0x1f8>;
> + reg-names = "dsi_ctrl";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <4>;
> +
> + assigned-clocks = <&mmcc BYTE0_CLK_SRC>, <&mmcc 
> PCLK0_CLK_SRC>;
> + assigned-clock-parents = <&dsi_phy0 0>, 
> <&dsi_phy0 1>;
One per line, please

> +
> + clocks = <&mmcc MDSS_MDP_CLK>,
> +  <&mmcc MDSS_AHB_CLK>,
> +  <&mmcc MDSS_AXI_CLK>,
> +  <&mmcc MDSS_BYTE0_CLK>,
> +  <&mmcc MDSS_PCLK0_CLK>,
> +  <&mmcc MDSS_ESC0_CLK>,
> +  <&mmcc MMSS_MISC_AHB_CLK>;
> + clock-names = "mdp_core",
> +   "iface",
> +   "bus",
> +   "byte",
> +   "pixel",
> +   "core",
> +   "core_mmss";
> +
> + phys = <&dsi_phy0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + dsi

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

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 11:44, Luca Weiss wrote:
> 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.
> 
> Signed-off-by: Luca Weiss 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  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[] = {
>   &apq8064_dsi_cfg, &msm_dsi_v2_host_ops},
>   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0,
>   &msm8974_apq8084_dsi_cfg, &msm_dsi_6g_host_ops},
> + {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0_2,
> + &msm8974_apq8084_dsi_cfg, &msm_dsi_6g_host_ops},
>   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1,
>   &msm8974_apq8084_dsi_cfg, &msm_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_00x1000
> +#define MSM_DSI_6G_VER_MINOR_V1_0_2  0x1002
>  #define MSM_DSI_6G_VER_MINOR_V1_10x1001
>  #define MSM_DSI_6G_VER_MINOR_V1_1_1  0x10010001
>  #define MSM_DSI_6G_VER_MINOR_V1_20x1002
> 


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

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 11:44, Luca Weiss wrote:
> Add the required config for the v1.1 MDP5 found on MSM8226.
> 
> 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 = {
Luca, this patch looks good as-is (without diving into the values).

Dmitry, I see some things that we may improve here..

1. Rename msm8xab to msm89ab or something, it's really inconsistent
   with other drivers

2. Some values seem very common / always constant.. perhaps we could
   add some #defines like we do in DPU?

3. Can we add some magic defines to make flush_hw_mask non-cryptic?

4. We can probably use pointers in data structs and deduplicate identical
   blocks!

Konrad
> + .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 = &msm8x74v1_config } },
> + { .revision = 1, .config = { .hw = &msm8x26_config } },
>   { .revision = 2, .config = { .hw = &msm8x74v2_config } },
>   { .revision = 3, .config = { .hw = &apq8084_config } },
>   { .revision = 6, .config = { .hw = &msm8x16_config } },
> 


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

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 11:44, Luca Weiss wrote:
> 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 = &dsi_phy_28nm_hpm_famb_cfgs },
>   { .compatible = "qcom,dsi-phy-28nm-lp",
> .data = &dsi_phy_28nm_lp_cfgs },
> + { .compatible = "qcom,dsi-phy-28nm-8226",
> +   .data = &dsi_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..f71308387566 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_LPBIT(0)
> +#define DSI_PHY_28NM_QUIRK_PHY_8226  BIT(1)
>  
>  #define LPFR_LUT_SIZE10
>  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 = &pll_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; // 1
Did you send the correct revision?

Konrad
> + 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; // 4
> + 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; // 2
> + val |= DSI_28nm_PHY_PLL_GLB_CFG_PLL_ENABLE; // 8
> + 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);
> +
> + 

Re: [Freedreno] [PATCH 0/5] MDSS reg bus interconnect

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 12:08, Konrad Dybcio wrote:



On 29.05.2023 10:47, Dmitry Baryshkov wrote:

On 29/05/2023 10:42, Konrad Dybcio wrote:



On 29.05.2023 04:42, Dmitry Baryshkov wrote:

On Mon, 17 Apr 2023 at 18:30, Konrad Dybcio  wrote:


Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

This series tries to address the lack of that.

Example path:

interconnects = <&bimc MASTER_AMPSS_M0 0 &config_noc SLAVE_DISPLAY_CFG 0>;


If we are going to touch the MDSS interconnects, could you please also
add the rotator interconnect to the bindings?
We do not need to touch it at this time, but let's not have to change
bindings later again.


Ack


Also, several points noted from the mdss fbdev driver:

- All possible clents vote for the low bw setting. This includes DSI, HDMI, 
MDSS itself and INTF

As in, "you need NUM_CLIENTS * MIN_VOTE" or as in "any client necessitates
a vote"?


Each client has separate vote




- SMMU also casts such vote, which I do not think should be necessary, unless 
there is a separate MDSS SMMU?

There's one on 8996, pre-845 SoCs often have a MMSS MMU, 845 and
later have a MMSS-specific TBU which (theoretically) requires a
vote for access to 0x400-0x7ff SIDs


Ack.




- PINGPONG cacsts high bw setting for the sake of speeding up the LUT tables if 
required.

Hm, I think is would be a separate topic.


I think so. I'd do a single vote from mdp5/dpu1. Then we can cast higher 
vote from PP/DSPP/etc.


--
With best wishes
Dmitry



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

2023-05-29 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.

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[] 
= {
&apq8064_dsi_cfg, &msm_dsi_v2_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0,
&msm8974_apq8084_dsi_cfg, &msm_dsi_6g_host_ops},
+   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0_2,
+   &msm8974_apq8084_dsi_cfg, &msm_dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1,
&msm8974_apq8084_dsi_cfg, &msm_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 1/7] dt-bindings: msm: dsi-phy-28nm: Document msm8226 compatible

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

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

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml 
b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
index cf4a338c4661..bd70c3873ca9 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml
@@ -18,6 +18,7 @@ properties:
   - qcom,dsi-phy-28nm-hpm
   - qcom,dsi-phy-28nm-hpm-fam-b
   - qcom,dsi-phy-28nm-lp
+  - qcom,dsi-phy-28nm-8226
   - qcom,dsi-phy-28nm-8960
 
   reg:
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 2/7] dt-bindings: display/msm: dsi-controller-main: Add msm8226 compatible

2023-05-29 Thread Luca Weiss
Add the compatible for the DSI found on MSM8226.

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 0/7] Display support for MSM8226

2023-05-29 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 
---
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 |   1 +
 .../devicetree/bindings/display/msm/qcom,mdp5.yaml |   1 +
 .../devicetree/bindings/display/msm/qcom,mdss.yaml |   1 +
 arch/arm/boot/dts/qcom-msm8226.dtsi| 118 +
 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, 309 insertions(+), 1 deletion(-)
---
base-commit: e5c87df1b3ab5220362ec48f907cc62ba8928b01
change-id: 20230308-msm8226-mdp-6431e8d672a0

Best regards,
-- 
Luca Weiss 



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

2023-05-29 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 | 118 
 1 file changed, 118 insertions(+)

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

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

2023-05-29 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 = &dsi_phy_28nm_hpm_famb_cfgs },
{ .compatible = "qcom,dsi-phy-28nm-lp",
  .data = &dsi_phy_28nm_lp_cfgs },
+   { .compatible = "qcom,dsi-phy-28nm-8226",
+ .data = &dsi_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..f71308387566 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 = &pll_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; // 1
+   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; // 4
+   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; // 2
+   val |= DSI_28nm_PHY_PLL_GLB_CFG_PLL_ENABLE; // 8
+   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 = DS

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

2023-05-29 Thread Luca Weiss
Add the compatible for the MDP5 found on MSM8226.

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 4/7] drm/msm/mdp5: Add MDP5 configuration for MSM8226

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

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 = &msm8x74v1_config } },
+   { .revision = 1, .config = { .hw = &msm8x26_config } },
{ .revision = 2, .config = { .hw = &msm8x74v2_config } },
{ .revision = 3, .config = { .hw = &apq8084_config } },
{ .revision = 6, .config = { .hw = &msm8x16_config } },

-- 
2.40.1



Re: [Freedreno] [PATCH 0/5] MDSS reg bus interconnect

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 10:47, Dmitry Baryshkov wrote:
> On 29/05/2023 10:42, Konrad Dybcio wrote:
>>
>>
>> On 29.05.2023 04:42, Dmitry Baryshkov wrote:
>>> On Mon, 17 Apr 2023 at 18:30, Konrad Dybcio  
>>> wrote:

 Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
 another path that needs to be handled to ensure MDSS functions properly,
 namely the "reg bus", a.k.a the CPU-MDSS interconnect.

 Gating that path may have a variety of effects.. from none to otherwise
 inexplicable DSI timeouts..

 This series tries to address the lack of that.

 Example path:

 interconnects = <&bimc MASTER_AMPSS_M0 0 &config_noc SLAVE_DISPLAY_CFG 0>;
>>>
>>> If we are going to touch the MDSS interconnects, could you please also
>>> add the rotator interconnect to the bindings?
>>> We do not need to touch it at this time, but let's not have to change
>>> bindings later again.
>>>
>> Ack
> 
> Also, several points noted from the mdss fbdev driver:
> 
> - All possible clents vote for the low bw setting. This includes DSI, HDMI, 
> MDSS itself and INTF
As in, "you need NUM_CLIENTS * MIN_VOTE" or as in "any client necessitates
a vote"?

> - SMMU also casts such vote, which I do not think should be necessary, unless 
> there is a separate MDSS SMMU?
There's one on 8996, pre-845 SoCs often have a MMSS MMU, 845 and
later have a MMSS-specific TBU which (theoretically) requires a
vote for access to 0x400-0x7ff SIDs

> - PINGPONG cacsts high bw setting for the sake of speeding up the LUT tables 
> if required.
Hm, I think is would be a separate topic.

Konrad
> 


Re: [Freedreno] [PATCH 0/5] MDSS reg bus interconnect

2023-05-29 Thread Dmitry Baryshkov

On 29/05/2023 10:42, Konrad Dybcio wrote:



On 29.05.2023 04:42, Dmitry Baryshkov wrote:

On Mon, 17 Apr 2023 at 18:30, Konrad Dybcio  wrote:


Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

This series tries to address the lack of that.

Example path:

interconnects = <&bimc MASTER_AMPSS_M0 0 &config_noc SLAVE_DISPLAY_CFG 0>;


If we are going to touch the MDSS interconnects, could you please also
add the rotator interconnect to the bindings?
We do not need to touch it at this time, but let's not have to change
bindings later again.


Ack


Also, several points noted from the mdss fbdev driver:

- All possible clents vote for the low bw setting. This includes DSI, 
HDMI, MDSS itself and INTF
- SMMU also casts such vote, which I do not think should be necessary, 
unless there is a separate MDSS SMMU?
- PINGPONG cacsts high bw setting for the sake of speeding up the LUT 
tables if required.


--
With best wishes
Dmitry



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

2023-05-29 Thread Manivannan Sadhasivam
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 &gmu, as it shouldn't have been used.
> >>> - Added missing compatible to &adreno_smmu.
> >>> - Dropped aoss_qmp clock in &gmu and &adreno_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 = <&adreno_smmu 0 0xc00>, <&adreno_smmu 1 0xc00>;
> >>> + operating-points-v2 = <&gpu_opp_table>;
> >>> +
> >>> + qcom,gmu = <&gmu>;
> >>> + interconnects = <&gem_noc MASTER_GFX3D 0 &mc_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";
> >>> +

Re: [Freedreno] [PATCH 0/5] MDSS reg bus interconnect

2023-05-29 Thread Konrad Dybcio



On 29.05.2023 04:42, Dmitry Baryshkov wrote:
> On Mon, 17 Apr 2023 at 18:30, Konrad Dybcio  wrote:
>>
>> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
>> another path that needs to be handled to ensure MDSS functions properly,
>> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
>>
>> Gating that path may have a variety of effects.. from none to otherwise
>> inexplicable DSI timeouts..
>>
>> This series tries to address the lack of that.
>>
>> Example path:
>>
>> interconnects = <&bimc MASTER_AMPSS_M0 0 &config_noc SLAVE_DISPLAY_CFG 0>;
> 
> If we are going to touch the MDSS interconnects, could you please also
> add the rotator interconnect to the bindings?
> We do not need to touch it at this time, but let's not have to change
> bindings later again.
> 
Ack

Konrad
>>
>> Signed-off-by: Konrad Dybcio 
>> ---
>> Konrad Dybcio (5):
>>   dt-bindings: display/msm: Add reg bus interconnect
>>   drm/msm/dpu1: Rename path references to mdp_path
>>   drm/msm/mdss: Rename path references to mdp_path
>>   drm/msm/mdss: Handle the reg bus ICC path
>>   drm/msm/dpu1: Handle the reg bus ICC path
>>
>>  .../bindings/display/msm/mdss-common.yaml  |  1 +
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c  | 10 +++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 34 
>> -
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h|  5 ++--
>>  drivers/gpu/drm/msm/msm_mdss.c | 35 
>> ++
>>  5 files changed, 57 insertions(+), 28 deletions(-)
>> ---
>> base-commit: d3f2cd24819158bb70701c3549e586f9df9cee67
>> change-id: 20230417-topic-dpu_regbus-abc94a770952
>>
>> Best regards,
>> --
>> Konrad Dybcio 
>>
> 
> 


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

2023-05-29 Thread Konrad Dybcio



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 &gmu, as it shouldn't have been used.
>>> - Added missing compatible to &adreno_smmu.
>>> - Dropped aoss_qmp clock in &gmu and &adreno_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 = <&adreno_smmu 0 0xc00>, <&adreno_smmu 1 0xc00>;
>>> +   operating-points-v2 = <&gpu_opp_table>;
>>> +
>>> +   qcom,gmu = <&gmu>;
>>> +   interconnects = <&gem_noc MASTER_GFX3D 0 &mc_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 = ,
>>> +;
>>> +   interrupt-names = "hfi", "gmu";
>>> +   clocks = <&gpucc GPU_CC_CX_GMU_CLK>,