intel / ipu6-drivers

152 stars 49 forks source link

Fix Galaxy Book4 Pro compatibility #230

Closed arter97 closed 1 month ago

arter97 commented 1 month ago

Galaxy Book4 Pro uses OV02C10, and it needed some additional fixes to make it work.

hao-yao commented 1 month ago

I'm also doing similar changes, but I didn't know that there are also hardware configurations like "ov02c10 + discrete power control" without IVSC. This version looks good, but let me verify if it works on my side.

By the way, how do you think use dev_dbg instead of dev_info in commit "print module name"? dev_info maybe too noisy.

arter97 commented 1 month ago

I'm also doing similar changes, but I didn't know that there are also hardware configurations like "ov02c10 + discrete power control" without IVSC. This version looks good, but let me verify if it works on my side.

Sure thing :)

By the way, how do you think use dev_dbg instead of dev_info in commit "print module name"? dev_info maybe too noisy.

Yup, that makes sense. I've updated the PR.

hao-yao commented 1 month ago

Thanks @arter97 , I merged my changes to your patches and pushed here and a simple test on my side is good. Could you help review that and test if it works for you?

Currently ov02c10 has been used in many devices with IPU6 in different hardware configurations. I think we'd better review and test more before we merge this PR.

arter97 commented 1 month ago

Yup, it works correctly on my end too.

I noticed that this part is missing though:

#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 6, 0) && \
    IS_ENABLED(CONFIG_INTEL_VSC)
    ov02c10->mipi_lanes = OV02C10_DATA_LANES;
    ov02c10->conf.lane_num = ov02c10->mipi_lanes;
    /* frequency unit 100k */
    ov02c10->conf.freq = OV02C10_LINK_FREQ_400MHZ / 100000;
    ret = vsc_acquire_camera_sensor(&ov02c10->conf,
                    ov02c10_vsc_privacy_callback,
                    ov02c10, &ov02c10->status);
    if (ret == -EAGAIN) {
        return -EPROBE_DEFER;
    } else if (ret) {
        dev_err(&client->dev, "Acquire VSC failed");
        return ret;
    }
#endif

I'm assuming you'd know better but just wanted to point that out if deleting that was a mistake.

Regardless, it's all working fine on my end :)

Let me know if you want me to update this PR to your latest tree directly.

hao-yao commented 1 month ago

Thanks,

#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 6, 0) && \
    IS_ENABLED(CONFIG_INTEL_VSC)
  ov02c10->mipi_lanes = OV02C10_DATA_LANES;
  ov02c10->conf.lane_num = ov02c10->mipi_lanes;
  /* frequency unit 100k */
  ov02c10->conf.freq = OV02C10_LINK_FREQ_400MHZ / 100000;
  ret = vsc_acquire_camera_sensor(&ov02c10->conf,
                  ov02c10_vsc_privacy_callback,
                  ov02c10, &ov02c10->status);
  if (ret == -EAGAIN) {
      return -EPROBE_DEFER;
  } else if (ret) {
      dev_err(&client->dev, "Acquire VSC failed");
      return ret;
  }
#endif

I'm assuming you'd know better but just wanted to point that out if deleting that was a mistake.

This part is moved to ov02c10_power_on() so we can replace this by ov02c10_power_on().

Let me know if you want me to update this PR to your latest tree directly.

I need to do more test on my side, maybe my branch still need some changes. Will let you know if everything works well.

hao-yao commented 1 month ago

@arter97 a few fixes here: https://github.com/hao-yao/ipu6-drivers/tree/fix-ov02c Could you help copy my branch to yours and update this PR? I think we can merge now.

arter97 commented 1 month ago

Yup, all done.

Tested it and it works well on my laptop as well. Thanks! :)

matinone commented 3 weeks ago

Awesome work here @arter97 @hao-yao ! If I wanted to test this, is it enough to follow the build instructions from the README, or would I need to do anything else? Thanks!

arter97 commented 3 weeks ago

Nothing special than any other IPU6 setup, it differs from distros to distros so you may have to google a bit.

IPU6 is generally just hard to get it right unlike traditional UVC webcams.