Re: [PATCH 12/13] media: qcom: camss: Add sm8550 support

From: Depeng Shao
Date: Thu Jul 11 2024 - 10:37:00 EST


Hi Bryan,


On 7/10/2024 8:02 PM, Bryan O'Donoghue wrote:
On 09/07/2024 17:06, Depeng Shao wrote:


diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index 858db5d4ca75..90fba25db4c6 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -1013,6 +1013,7 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
  {
      struct device *dev = camss->dev;
      struct platform_device *pdev = to_platform_device(dev);
+    struct resource *top_res;
      int i, j;
      int ret;
@@ -1040,6 +1041,26 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
          else
              csid->base = csid->res->parent_dev_ops->get_base_address(camss, id)
                   + VFE_480_CSID_OFFSET;
+    } else if (camss->res->version == CAMSS_8550) {
+        /* for titan 780, CSID lite registers are inside the VFE lite region,
+         * between the VFE "top" and "bus" registers. this requires
+         * VFE to be initialized before CSID
+         */
+        if (csid_is_lite(csid))
+            csid->base = csid->res->parent_dev_ops->get_base_address(camss, id);
+        else {
+            csid->base = devm_platform_ioremap_resource_byname(pdev, res->reg[0]);
+            /* CSID "top" is a new function in Titan780.
+             * CSID can connect to VFE & SFE(Sensor Front End).
+             * This connection is ontrolled by CSID "top" registers.
+             * CSID "top" registers at only one region.
+             */
+            top_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, res->reg[1]);
+            csid->top_base = ioremap(top_res->start, resource_size(top_res));
+        }

If we just specify the csid lite regs in the dts, we don't need custom code to get a pointer to them.


Yes, we can get the csid lite reg directly, will update this part.

Similarly the csid->top_base = () should be generic not SoC specific, i.e. we should be able to add in CSID 980 without adding any custom code to camss-csid.c


csid lite doesn't have top reg block, but you are right, this can be generic, we can get csid->top_base if res->reg[1] isn't NULL.


diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 83c5a36d071f..479474c1cd95 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -338,6 +338,7 @@ static u32 vfe_src_pad_code(struct vfe_line *line, u32 sink_code,
      case CAMSS_845:
      case CAMSS_8250:
      case CAMSS_8280XP:
+    case CAMSS_8550:
          switch (sink_code) {
          case MEDIA_BUS_FMT_YUYV8_1X16:
          {
@@ -408,6 +409,10 @@ int vfe_reset(struct vfe_device *vfe)
      reinit_completion(&vfe->reset_complete);
+    /* The reset has been moved to csid in 8550 */
+    if (vfe->camss->res->version == CAMSS_8550)
+        return 0;
+

Custom code for a specific SoC in camss.c camss-csid.c or camss-vfe.c indicates to me we need to do more work elsewhere.

This would do the same job for you.

static void vfe_global_reset(struct vfe_device *vfe)
{
        /* VFE780 has no global reset, simply report a completion */
        complete(&vfe->reset_complete);
}

const struct vfe_hw_ops vfe_ops_780 = {
        .global_reset = vfe_global_reset,

      vfe->res->hw_ops->global_reset(vfe);
      time = wait_for_completion_timeout(&vfe->reset_complete

Sure, I will optimize this part.


Thanks,
Depeng