Re: [PATCH v3 14/15] media: venus: Convert to use resource-managed OPP API

[Stanimir Varbanov]

Hi,

On 3/14/21 6:34 PM, Dmitry Osipenko wrote:
> From: Yangtao Li 
> 
> Use resource-managed OPP API to simplify code.
> 
> Signed-off-by: Yangtao Li 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/media/platform/qcom/venus/core.h  |  1 -
>  .../media/platform/qcom/venus/pm_helpers.c| 35 +--
>  2 files changed, 8 insertions(+), 28 deletions(-)


I'll take this through media-tree once OPP API changes are merged.

-- 
regards,
Stan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/19] drm: add msm compressed format modifiers

[Stanimir Varbanov]

On 07/26/2018 02:09 AM, Jeykumar Sankaran wrote:
> On 2018-07-25 07:09, Stanimir Varbanov wrote:
>> Hi,
>>
>> On 07/20/2018 11:42 PM, Sean Paul wrote:
>>> From: Jeykumar Sankaran 
>>>
>>> Qualcomm Snapdragon chipsets uses compressed format
>>> to optimize BW across multiple IP's. This change adds
>>> needed modifier support in drm for a simple 4x4 tile
>>> based compressed variants of base formats.
>>>
>>> Changes in v3:
>>> - Removed duplicate entry for DRM_FORMAT_MOD_QCOM_COMPRESSED (Rob Clark)
>>>
>>> Signed-off-by: Jeykumar Sankaran 
>>> Signed-off-by: Sean Paul 
>>> ---
>>>  include/uapi/drm/drm_fourcc.h | 37 +++
>>>  1 file changed, 37 insertions(+)
>>>
>>> diff --git a/include/uapi/drm/drm_fourcc.h
>>> b/include/uapi/drm/drm_fourcc.h
>>> index e04613d30a13..1c9a6bf8c81e 100644
>>> --- a/include/uapi/drm/drm_fourcc.h
>>> +++ b/include/uapi/drm/drm_fourcc.h
>>> @@ -298,6 +298,43 @@ extern "C" {
>>>   */
>>>  #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE   
>>> fourcc_mod_code(SAMSUNG, 1)
>>>
>>> +/*
>>> + * Qualcomm Compressed Format
>>> + *
>>> + * Refers to a compressed variant of the base format that is
>>> compressed.
>>> + * Implementation may be platform and base-format specific.
>>> + *
>>> + * Each macrotile consists of m x n (mostly 4 x 4) tiles.
>>> + * Pixel data pitch/stride is aligned with macrotile width.
>>> + * Pixel data height is aligned with macrotile height.
>>> + * Entire pixel data buffer is aligned with 4k(bytes).
>>> + */
>>> +#define DRM_FORMAT_MOD_QCOM_COMPRESSED    fourcc_mod_code(QCOM, 1)
>>> +
>>> +/*
>>> + * QTI DX Format
>>> + *
>>> + * Refers to a DX variant of the base format.
>>> + * Implementation may be platform and base-format specific.
>>> + */
>>> +#define DRM_FORMAT_MOD_QCOM_DX    fourcc_mod_code(QCOM, 0x2)
>>
>> What DX stands for?
> 
> _DX is QCOM modifier for identifying 10bit version of NV12 format since
> there is no fourcc_code for the same.
> 

Did you mean such as COLOR_FMT_P010? And how DX is translated to ?

-- 
regards,
Stan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 03/19] drm: add msm compressed format modifiers

[Stanimir Varbanov]

Hi,

On 07/20/2018 11:42 PM, Sean Paul wrote:
> From: Jeykumar Sankaran 
> 
> Qualcomm Snapdragon chipsets uses compressed format
> to optimize BW across multiple IP's. This change adds
> needed modifier support in drm for a simple 4x4 tile
> based compressed variants of base formats.
> 
> Changes in v3:
> - Removed duplicate entry for DRM_FORMAT_MOD_QCOM_COMPRESSED (Rob Clark)
> 
> Signed-off-by: Jeykumar Sankaran 
> Signed-off-by: Sean Paul 
> ---
>  include/uapi/drm/drm_fourcc.h | 37 +++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index e04613d30a13..1c9a6bf8c81e 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -298,6 +298,43 @@ extern "C" {
>   */
>  #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILEfourcc_mod_code(SAMSUNG, 1)
>  
> +/*
> + * Qualcomm Compressed Format
> + *
> + * Refers to a compressed variant of the base format that is compressed.
> + * Implementation may be platform and base-format specific.
> + *
> + * Each macrotile consists of m x n (mostly 4 x 4) tiles.
> + * Pixel data pitch/stride is aligned with macrotile width.
> + * Pixel data height is aligned with macrotile height.
> + * Entire pixel data buffer is aligned with 4k(bytes).
> + */
> +#define DRM_FORMAT_MOD_QCOM_COMPRESSED   fourcc_mod_code(QCOM, 1)
> +
> +/*
> + * QTI DX Format
> + *
> + * Refers to a DX variant of the base format.
> + * Implementation may be platform and base-format specific.
> + */
> +#define DRM_FORMAT_MOD_QCOM_DX   fourcc_mod_code(QCOM, 0x2)

What DX stands for?


-- 
regards,
Stan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] firmware: qcom_scm: Add set remote state API

[Stanimir Varbanov]

Andy,

Could you queue this patch for 4.11?

On 01/17/2017 07:56 AM, Andy Gross wrote:
> This patch adds a set remote state SCM API.  This will be used by the
> Venus and GPU subsystems to set state on the remote processors.
> 
> This work was based on two patch sets by Jordan Crouse and Stanimir
> Varbanov.
> 
> Signed-off-by: Andy Gross <andy.gr...@linaro.org>
> ---
>  drivers/firmware/qcom_scm-32.c | 18 ++
>  drivers/firmware/qcom_scm-64.c | 16 
>  drivers/firmware/qcom_scm.c|  6 ++
>  drivers/firmware/qcom_scm.h|  2 ++
>  include/linux/qcom_scm.h   |  4 +++-
>  5 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index c6aeedb..8ad226c 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -560,3 +560,21 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool 
> reset)
>  
>   return ret ? : le32_to_cpu(out);
>  }
> +
> +int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id)
> +{
> + struct {
> + __le32 state;
> + __le32 id;
> + } req;
> + __le32 scm_ret = 0;
> + int ret;
> +
> + req.state = cpu_to_le32(state);
> + req.id = cpu_to_le32(id);
> +
> + ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_SET_REMOTE_STATE,
> + , sizeof(req), _ret, sizeof(scm_ret));
> +
> + return ret ? : le32_to_cpu(scm_ret);
> +}
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 4a0f5ea..4b220ab 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -358,3 +358,19 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool 
> reset)
>  
>   return ret ? : res.a1;
>  }
> +
> +int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 id)
> +{
> + struct qcom_scm_desc desc = {0};
> + struct arm_smccc_res res;
> + int ret;
> +
> + desc.args[0] = state;
> + desc.args[1] = id;
> + desc.arginfo = QCOM_SCM_ARGS(2);
> +
> + ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_SET_REMOTE_STATE,
> + , );
> +
> + return ret ? : res.a1;
> +}
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 65d0d9d..d987bcc 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -324,6 +324,12 @@ bool qcom_scm_is_available(void)
>  }
>  EXPORT_SYMBOL(qcom_scm_is_available);
>  
> +int qcom_scm_set_remote_state(u32 state, u32 id)
> +{
> + return __qcom_scm_set_remote_state(__scm->dev, state, id);
> +}
> +EXPORT_SYMBOL(qcom_scm_set_remote_state);
> +
>  static int qcom_scm_probe(struct platform_device *pdev)
>  {
>   struct qcom_scm *scm;
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index 3584b00..6a0f154 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -15,6 +15,8 @@
>  #define QCOM_SCM_SVC_BOOT0x1
>  #define QCOM_SCM_BOOT_ADDR   0x1
>  #define QCOM_SCM_BOOT_ADDR_MC0x11
> +#define QCOM_SCM_SET_REMOTE_STATE0xa
> +extern int __qcom_scm_set_remote_state(struct device *dev, u32 state, u32 
> id);
>  
>  #define QCOM_SCM_FLAG_HLOS   0x01
>  #define QCOM_SCM_FLAG_COLDBOOT_MC0x02
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 7e004fb..d32f6f1 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -39,6 +39,7 @@ extern int qcom_scm_pas_mem_setup(u32 peripheral, 
> phys_addr_t addr,
>  extern int qcom_scm_pas_shutdown(u32 peripheral);
>  extern void qcom_scm_cpu_power_down(u32 flags);
>  extern u32 qcom_scm_get_version(void);
> +extern int qcom_scm_set_remote_state(u32 state, u32 id);
>  #else
>  static inline
>  int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
> @@ -64,6 +65,7 @@ static inline int qcom_scm_pas_mem_setup(u32 peripheral, 
> phys_addr_t addr,
>  static inline int qcom_scm_pas_shutdown(u32 peripheral) { return -ENODEV; }
>  static inline void qcom_scm_cpu_power_down(u32 flags) {}
>  static inline u32 qcom_scm_get_version(void) { return 0; }
> +static inline u32
> +qcom_scm_set_remote_state(u32 state,u32 id) { return -ENODEV; }
>  #endif
> -
>  #endif
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] firmware: qcom_scm: Add set remote state API

[Stanimir Varbanov]

Thanks Andy,

On 01/17/2017 07:56 AM, Andy Gross wrote:
> This patch adds a set remote state SCM API.  This will be used by the
> Venus and GPU subsystems to set state on the remote processors.
> 
> This work was based on two patch sets by Jordan Crouse and Stanimir
> Varbanov.
> 
> Signed-off-by: Andy Gross <andy.gr...@linaro.org>
> ---
>  drivers/firmware/qcom_scm-32.c | 18 ++
>  drivers/firmware/qcom_scm-64.c | 16 
>  drivers/firmware/qcom_scm.c|  6 ++
>  drivers/firmware/qcom_scm.h|  2 ++
>  include/linux/qcom_scm.h   |  4 +++-
>  5 files changed, 45 insertions(+), 1 deletion(-)

Acked-by: Stanimir Varbanov <stanimir.varba...@linaro.org>


-- 
regards,
Stan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Freedreno] [PATCH 10/12] firmware: qcom_scm: Add qcom_scm_gpu_zap_resume()

[Stanimir Varbanov]

Hi Andy,

On 01/15/2017 07:20 AM, Andy Gross wrote:
> + Stanimir
> 
> On Sat, Jan 14, 2017 at 09:49:01PM -0600, Andy Gross wrote:
>> On Fri, Jan 13, 2017 at 04:24:38PM -0700, Jordan Crouse wrote:
>>> On Fri, Jan 13, 2017 at 11:12:41AM -0600, Andy Gross wrote:
 On Mon, Nov 28, 2016 at 12:28:35PM -0700, Jordan Crouse wrote:
> Add an interface to trigger the remote processor to reinitialize the GPU
> zap shader on power-up.
>
> Signed-off-by: Jordan Crouse 
> ---

 

> +int __qcom_scm_gpu_zap_resume(struct device *dev)
> +{
> + struct qcom_scm_desc desc = {0};
> + struct arm_smccc_res res;
> + int ret;
> +
> + desc.args[0] = 0;
>>>
>>> This is an opcode to force the state to resume.
>>>
>>> QCOM_SCM_BOOT_SET_STATE_RESUME perhaps?  Or something similar but shorter.
>>>
> + desc.args[1] = 13;
>>>
>>> This is the same as the SCM id of the GPU but I think that is a coincidence.
>>> We've always used it to identify the GPU in this call.
>>>
>>> QCOM_SCM_BOOT_SET_STATE_GPU would be fine here - or something similar.
>>>
 Can I get a define here for these two?  Or maybe a comment on what these 
 values
 are?

> + desc.arginfo = QCOM_SCM_ARGS(2);
> +
> + ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, 0x0A, , );

 Same with the 0xA.  We usually throw a #define in for the command 
 definitions.
>>>
>>> 0x0A sets the state of the device - for us it is always 0 (resume) and 
>>> always
>>> the GPU.
>>>
>>> #define  QCOM_SCM_BOOT_SET_STATE 0x0A
>>>
 Otherwise this all looks fine.  If you can get back to me with either the 
 values
 or a new patch I can include this in the next pull.
>>>
>>> I'll make the changes and start the song and dance, but you'll no doubt be
>>> faster than I.
>>
>> I can just fix up the patch with the above.  Thanks for the additional 
>> details.
> 
> The plot thickens.  So I have a patch from Stanimir concerning another SCM 
> call
> that is using the same command and number of arguments.  And it also concerns
> setting state.  I think that we need to roll a common API for setting the 
> state
> and then both of you can call it.  That way we can kill two birds with one
> stone.
> 
> Something along the lines of a function prototype:
> int qcom_scm_set_remote_state(u32 state, u32 id)
> {
>   return __qcom_scm_set_remote_state(__scm->dev, state, id);
> }
> EXPORT_SYMBOL(qcom_scm_set_remote_state);
> 
> where state is the state you want set, and id is the identifier of the remote
> proc.
> 
> Does this make sense for both of your use cases?

I'm fine with that.

-- 
regards,
Stan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/msm: add OCMEM driver

[Stanimir Varbanov]

On 09/30/2015 02:45 PM, Rob Clark wrote:
> On Wed, Sep 30, 2015 at 7:31 AM, Rob Clark  wrote:
>> On Wed, Sep 30, 2015 at 3:51 AM, Stanimir Varbanov
>>  wrote:
>>> Hi Rob,
>>>
>>> Thanks for your work.
>>>
>>> On 09/29/2015 10:48 PM, Rob Clark wrote:
>>>> For now, since the GPU is the only upstream consumer, just stuff this
>>>> into drm/msm.  Eventually if we have other consumers, we'll have to
>>>
>>> As the video encoder/decoder driver (vidc) for apq8084 && msm8974 also
>>> use the ocmem for scratch buffers, it might be better to relocate the
>>> ocmem driver in drivers/soc/qcom from the beginning?
>>
>> I wasn't really sure how soon vidc would support 8084/8974 (I assume
>> first target is 8916 which fortunately doesn't have ocmem), or if it

My expectations are that the same driver will work on apq8084, as well.

>> was mandatory or just power optimization..  but yes, the plan was to
>> move it somewhere else (not sure quite where, drivers/doc/qcom?)
>> sometime..  Preferably when someone who understands all the other
>> ocmem use-cases better could figure out what we really need to add to
>> the driver.
>>
>> In downstream driver there is a lot of complexity that appears to be
>> in order to allow two clients to each allocate a portion of a macro
>> within a region (ie. aggregate_macro_state(), apply_macro_vote(),
>> etc), and I wanted to figure out if that is even a valid use-case
>> before trying to make ocmem something that could actually support
>> multiple clients.
>>
>> There is also some complexity about ensuring that if clients aren't
>> split up on region boundaries, that you don't have one client in
>> region asking for wide-mode and other for narrow-mode..
>> (switch_region_mode()) but maybe we could handle that by just
>> allocating wide from bottom and narrow from top.  Also seems to be
>> some craziness for allowing one client to pre-empt/evict another.. a
>> dm engine, etc, etc..
>>
>> All I know is gpu just statically allocates one big region aligned
>> chunk of ocmem, so I ignored the rest of the crazy (maybe or maybe not
>> hypothetical) use-cases for now...

OK, I will try to sort out ocmem use cases for vidc driver.

> 
> btw, I should add that I don't mind adding it in drivers/soc/qcom (or
> somewhere else?) to start with if others prefer, I just didn't want to
> give the wrong impression that it is ready yet for multiple clients.

I see. Then to avoid confusions you could remove all clients and keep
only graphics from ocmem_client enum.

> 
> All I know is the gpu uses it statically and is pretty much useless
> without ocmem, so for lack of understanding of the other use cases I
> just tried to add simply what the gpu needs..

Got it now, thanks.

-- 
regards,
Stan

[PATCH 6/6] drm/msm: add OCMEM driver

[Stanimir Varbanov]

Hi Rob,

Thanks for your work.

On 09/29/2015 10:48 PM, Rob Clark wrote:
> For now, since the GPU is the only upstream consumer, just stuff this
> into drm/msm.  Eventually if we have other consumers, we'll have to

As the video encoder/decoder driver (vidc) for apq8084 && msm8974 also
use the ocmem for scratch buffers, it might be better to relocate the
ocmem driver in drivers/soc/qcom from the beginning?

I'm working on vidc driver upstream version so it will be nice to test
ocmem driver from it, too.

> split this out and make the allocation less hard coded.  But I'll punt
> on that until I better understand the non-gpu uses-cases (and whether
> the allocation *really* needs to be as complicated as it is in the
> downstream driver).
> 
> Signed-off-by: Rob Clark 
> ---
>  drivers/gpu/drm/msm/Makefile  |   3 +-
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c |  19 +-
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c |  19 +-
>  drivers/gpu/drm/msm/msm_drv.c |   2 +
>  drivers/gpu/drm/msm/msm_gpu.h |   3 +
>  drivers/gpu/drm/msm/ocmem/ocmem.c | 399 
> ++
>  drivers/gpu/drm/msm/ocmem/ocmem.h |  46 
>  7 files changed, 463 insertions(+), 28 deletions(-)
>  create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.c
>  create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.h
> 



-- 
regards,
Stan