On Fri, 16 Dec 2022, Andrzej Hajda <andrzej.hajda@xxxxxxxxx> wrote:
> The helper makes the code more compact and readable.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
[snip]
> @@ -649,23 +611,18 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder,
> enum port port;
>
> if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> - u32 temp;
> + u32 temp = intel_dsi->pixel_overlap;
> if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
> - for_each_dsi_port(port, intel_dsi->ports) {
> - temp = intel_de_read(dev_priv,
> - MIPI_CTRL(port));
> - temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
> - intel_dsi->pixel_overlap <<
> - BXT_PIXEL_OVERLAP_CNT_SHIFT;
This can't possibly be correct to begin with.
I think it's supposed to be the logical
temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK;
temp |= intel_dsi->pixel_overlap << BXT_PIXEL_OVERLAP_CNT_SHIFT;
Nothing else makes sense, really.
I think I'd just fix that in a separate patch first, and then do the rmw
changes on top.
> - intel_de_write(dev_priv, MIPI_CTRL(port),
> - temp);
> - }
> + for_each_dsi_port(port, intel_dsi->ports)
> + intel_de_rmw(dev_priv, MIPI_CTRL(port),
> + BXT_PIXEL_OVERLAP_CNT_MASK &
> + ~(temp << BXT_PIXEL_OVERLAP_CNT_SHIFT),
> + 0);
> } else {
> - temp = intel_de_read(dev_priv, VLV_CHICKEN_3);
> - temp &= ~PIXEL_OVERLAP_CNT_MASK |
> - intel_dsi->pixel_overlap <<
> - PIXEL_OVERLAP_CNT_SHIFT;
> - intel_de_write(dev_priv, VLV_CHICKEN_3, temp);
Ditto here.
> + intel_de_rmw(dev_priv, VLV_CHICKEN_3,
> + PIXEL_OVERLAP_CNT_MASK &
> + ~(temp << PIXEL_OVERLAP_CNT_SHIFT),
> + 0);
> }
> }
>
Everything else in the patch checks out. With the above split out and
the rmw rebased,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
--
Jani Nikula, Intel Open Source Graphics Center
